Rui Paulo | 5 Oct 2006 17:32

Re: Refactoring Congestion Control (take 2)

Previously, I wrote:

> 4) I'm facing a problem with our sysctl API.
>
> 	# sysctl net.inet.tcp.congctl.selected
> 	net.inet.tcp.congctl.selected = newreno
> 	# sysctl -w net.inet.tcp.congctl.selected=reno
> 	sysctl: net.inet.tcp.congctl.selected: sysctl() failed with Cannot  
> allocate memory
> 	# sysctl net.inet.tcp.congctl.selected
> 	net.inet.tcp.congctl.selected = reno

I still have this problem. Any ideas?

--
Rui Paulo

YAMAMOTO Takashi | 6 Oct 2006 11:29
Picon

Re: Refactoring Congestion Control (take 2)

> Previously, I wrote:
> 
> > 4) I'm facing a problem with our sysctl API.
> >
> > 	# sysctl net.inet.tcp.congctl.selected
> > 	net.inet.tcp.congctl.selected = newreno
> > 	# sysctl -w net.inet.tcp.congctl.selected=reno
> > 	sysctl: net.inet.tcp.congctl.selected: sysctl() failed with Cannot  
> > allocate memory
> > 	# sysctl net.inet.tcp.congctl.selected
> > 	net.inet.tcp.congctl.selected = reno
> 
> I still have this problem. Any ideas?
> 
> --
> Rui Paulo

sysctl_createv(clog, 0, NULL, NULL,
		CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
		CTLTYPE_STRING, "selected",
		SYSCTL_DESCR("Selected Congestion Control Mechanism"),
		sysctl_tcp_congctl, 0, &tcp_congctl_global_name, 0,
		CTL_NET, pf, IPPROTO_TCP, congctl_node,
		CTL_CREATE, CTL_EOL);

should be:

		sysctl_tcp_congctl, 0, NULL, TCPCC_MAXLEN,

yes, our sysctl is too cryptic.
(Continue reading)

Rui Paulo | 6 Oct 2006 15:06

Re: Refactoring Congestion Control (take 2)


On Oct 6, 2006, at 10:29 AM, YAMAMOTO Takashi wrote:

> sysctl_createv(clog, 0, NULL, NULL,
> 		CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
> 		CTLTYPE_STRING, "selected",
> 		SYSCTL_DESCR("Selected Congestion Control Mechanism"),
> 		sysctl_tcp_congctl, 0, &tcp_congctl_global_name, 0,
> 		CTL_NET, pf, IPPROTO_TCP, congctl_node,
> 		CTL_CREATE, CTL_EOL);
>
> should be:
>
> 		sysctl_tcp_congctl, 0, NULL, TCPCC_MAXLEN,
>
> yes, our sysctl is too cryptic.

Thanks a lot for spotting this, it now works perfectly!

--
Rui Paulo

Rui Paulo | 7 Oct 2006 22:59

Re: Refactoring Congestion Control (take 2)

I'm going to commit:
http://www.netbsd.org/~rpaulo/tcp_congctl.diff

in the upcoming week if there are no objections.
The setsockopt() handling is not done yet, but I'm planing to add  
that feature later.

--
Rui Paulo

YAMAMOTO Takashi | 8 Oct 2006 11:01
Picon

Re: Refactoring Congestion Control (take 2)

> I'm going to commit:
> http://www.netbsd.org/~rpaulo/tcp_congctl.diff
> 
> in the upcoming week if there are no objections.
> The setsockopt() handling is not done yet, but I'm planing to add  
> that feature later.
> 
> --
> Rui Paulo

"cwnd_inflation" sounds weird to me, given that what it does is
ack handling for fast recovery.  otoh, "new_data_acked" inflates cwnd.
isn't it better to unify these two callbacks?

btw, where sack will be in the whole picture, eventually?

YAMAMOTO Takashi

Rui Paulo | 8 Oct 2006 14:19

Re: Refactoring Congestion Control (take 2)


On Oct 8, 2006, at 10:01 AM, YAMAMOTO Takashi wrote:

> "cwnd_inflation" sounds weird to me, given that what it does is
> ack handling for fast recovery.  otoh, "new_data_acked" inflates cwnd.
> isn't it better to unify these two callbacks?

Well, "inflation" is probably not the best word, but I don't see why  
we want to unify them. What do you have in mind?

> btw, where sack will be in the whole picture, eventually?

I don't know yet. SACK, like ECN, can work with Reno, NewReno and  
other congestion control algorithms. At least, I'm sure we don't want:
congctl.available = reno newreno reno-sack newreno-sack, etc.

--
Rui Paulo

YAMAMOTO Takashi | 8 Oct 2006 15:49
Picon

Re: Refactoring Congestion Control (take 2)

> > "cwnd_inflation" sounds weird to me, given that what it does is
> > ack handling for fast recovery.  otoh, "new_data_acked" inflates cwnd.
> > isn't it better to unify these two callbacks?
> 
> Well, "inflation" is probably not the best word, but I don't see why  
> we want to unify them. What do you have in mind?

because both of them are called sequentially in most cases
(unless rcvacktoomuch, for which we can't do much anyway),
it isn't clear for me what's the benefit to have two callbacks
rather than one.

YAMAMOTO Takashi

Rui Paulo | 8 Oct 2006 16:04

Re: Refactoring Congestion Control (take 2)


On Oct 8, 2006, at 2:49 PM, YAMAMOTO Takashi wrote:

>>> "cwnd_inflation" sounds weird to me, given that what it does is
>>> ack handling for fast recovery.  otoh, "new_data_acked" inflates  
>>> cwnd.
>>> isn't it better to unify these two callbacks?
>>
>> Well, "inflation" is probably not the best word, but I don't see why
>> we want to unify them. What do you have in mind?
>
> because both of them are called sequentially in most cases
> (unless rcvacktoomuch, for which we can't do much anyway),
> it isn't clear for me what's the benefit to have two callbacks
> rather than one.

Right now, I don't see a way to have just one callback because of SACK.
We would have to call tcp_sack_newack() in every other  
tcp_xxxx_cwnd_inflation().
The point here is to introduce HSTCP and Westwood+ after these changes.

--
Rui Paulo

Martin Husemann | 8 Oct 2006 16:33
Picon

Use of sun_len in AF_UNIX socket addresses

Looking at PR lib/34744, I was not sure if we expect userland to set
sun_len correctly. A quick grep through our tree shows about 10 out of about
20 source files doing it, the others seem to leave sun_len as zero.

The kernel cares about sun_len at very few places, and I fail to see the
big picture. Since apparently the userland parts that do not set sun_len
work just fine, I guess we could replace the few kernel usages of
sun_len with SUN_LEN()?

Maybe we could even get rid of the sun_len field completely? (But then it
might not be worth the compmatibility issues - and of course I might just
be missing something.)

Martin

YAMAMOTO Takashi | 8 Oct 2006 16:39
Picon

Re: Refactoring Congestion Control (take 2)

> On Oct 8, 2006, at 2:49 PM, YAMAMOTO Takashi wrote:
> 
> >>> "cwnd_inflation" sounds weird to me, given that what it does is
> >>> ack handling for fast recovery.  otoh, "new_data_acked" inflates  
> >>> cwnd.
> >>> isn't it better to unify these two callbacks?
> >>
> >> Well, "inflation" is probably not the best word, but I don't see why
> >> we want to unify them. What do you have in mind?
> >
> > because both of them are called sequentially in most cases
> > (unless rcvacktoomuch, for which we can't do much anyway),
> > it isn't clear for me what's the benefit to have two callbacks
> > rather than one.
> 
> Right now, I don't see a way to have just one callback because of SACK.
> We would have to call tcp_sack_newack() in every other  
> tcp_xxxx_cwnd_inflation().

if you have sack, whatever xxxx is, tcp_xxxx_cwnd_inflation isn't
needed to be called?
(i'm not sure about the semantics of cwnd_inflation.)

> The point here is to introduce HSTCP and Westwood+ after these changes.

these callbacks should be separated for them, you mean?

YAMAMOTO Takashi

(Continue reading)


Gmane