* Re: [PATCH] xen/netfront: improve truesize tracking
From: Eric Dumazet @ 2013-01-07 15:26 UTC (permalink / raw)
To: Ian Campbell
Cc: Sander Eikelenboom, Rick Jones, netdev@vger.kernel.org,
Konrad Rzeszutek Wilk, annie li, xen-devel@lists.xensource.com
In-Reply-To: <1357568565.7989.96.camel@zakaz.uk.xensource.com>
On Mon, 2013-01-07 at 14:22 +0000, Ian Campbell wrote:
> On Mon, 2013-01-07 at 14:11 +0000, Sander Eikelenboom wrote:
> one is buggy.
> >
> > Will see if i can run against that as well, although i thought Eric
> > said to prefer the "skb->truesize += PAGE_SIZE *
> > skb_shinfo(skb)->nr_frags;"
>
> Right, I meant to compare "PAGE_SIZE * skb_shinfo(skb)->nr_frags" vs
> "skb->data_len - NETFRONT_SKB_CB(skb)->pull_to". TBH I trust Eric so I'm
> inclined to just go with what he suggests.
This is the more precise estimation, and it appears to not bring obvious
performance regression, so I definitely would favor this one ;)
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-07 15:32 UTC (permalink / raw)
To: Neil Horman
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <20130107144921.GA31577@hmsreliant.think-freely.org>
On 01/07/2013 09:49 AM, Neil Horman wrote:
> On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
>> Hello Neil,
>>
>> Le 12/15/12 02:22, Neil Horman a écrit :
>>> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
>>> algorithm selectable. This is all well and good, but Linus noted that it
>>> changes the default config:
>>> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>>>
>>> I've modified the sctp Kconfig file to reflect the recommended way of making
>>> this choice, using the thermal driver example specified, and brought the
>>> defaults back into line with the way they were prior to my origional patch
>>>
>>> Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
>>> algorithm, so we don't needlessly bloat the kernel by forcing a non-none
>>> default. This also led me to note that we won't honor the default none
>>> condition properly because of how sctp_net_init is encoded. Fix that up as
>>> well.
>>>
>>> Tested by myself (allbeit fairly quickly). All configuration combinations seems
>>> to work soundly.
>>>
>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>> CC: David Miller <davem@davemloft.net>
>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>> CC: linux-sctp@vger.kernel.org
>>> ---
>>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
>>> net/sctp/protocol.c | 4 ++--
>>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>> index a9edd2e..c262106 100644
>>> --- a/net/sctp/Kconfig
>>> +++ b/net/sctp/Kconfig
>>> @@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
>>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
>>>
>>> If unsure, say N
>>> +choice
>>> + prompt "Default SCTP cookie HMAC encoding"
>>> + default SCTP_COOKIE_HMAC_MD5
>>
>> Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
>> update to 3.8-rc2, and I usually build my kernel-headers with:
>>
>> yes '' | ARCH=foo make oldconfig
>>
>> and this just kept asking me for this config symbol because none
>> could be provided.
>> --
>> Florian
>>
>
> No, the config mechanism is setup to offer the user the ability to choose a
> default cookie hmac, alg, then optionally select any other hmac algs you would
> like to be made available (in the event you want to change the default at run
> time). When you select the default, it eables (via the select directive), the
> corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
> then prompts for the remaining values.
>
Neil
Actually, I think it should be as Florian suggests. The default value
of the choice should be one of the values defined as part of the choice
(the SCTP_DEFAULT_COOKIE_*). Turning on appropriate default would turn
on appropriate cookie config (SCTP_COOKIE_HMAC_*).
Would that save all the config trouble?
-vlad
> Neil
>
^ permalink raw reply
* [PATCH V2] xen/netfront: improve truesize tracking
From: Ian Campbell @ 2013-01-07 15:32 UTC (permalink / raw)
To: netdev
Cc: Ian Campbell, Sander Eikelenboom, Konrad Rzeszutek Wilk, annie li,
xen-devel
Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
than that. We have already accounted for this in
NETFRONT_SKB_CB(skb)->pull_to so use that instead.
Fixes WARN_ON from skb_try_coalesce.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: annie li <annie.li@oracle.com>
Cc: xen-devel@lists.xen.org
Cc: netdev@vger.kernel.org
Cc: stable@kernel.org # 3.7.x only
---
v2:
- acount frags as a full page as recommended by Eric.
---
drivers/net/xen-netfront.c | 27 ++++-----------------------
1 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index caa0110..b518538 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -959,29 +959,10 @@ err:
i = xennet_fill_frags(np, skb, &tmpq);
/*
- * Truesize approximates the size of true data plus
- * any supervisor overheads. Adding hypervisor
- * overheads has been shown to significantly reduce
- * achievable bandwidth with the default receive
- * buffer size. It is therefore not wise to account
- * for it here.
- *
- * After alloc_skb(RX_COPY_THRESHOLD), truesize is set
- * to RX_COPY_THRESHOLD + the supervisor
- * overheads. Here, we add the size of the data pulled
- * in xennet_fill_frags().
- *
- * We also adjust for any unused space in the main
- * data area by subtracting (RX_COPY_THRESHOLD -
- * len). This is especially important with drivers
- * which split incoming packets into header and data,
- * using only 66 bytes of the main data area (see the
- * e1000 driver for example.) On such systems,
- * without this last adjustement, our achievable
- * receive throughout using the standard receive
- * buffer size was cut by 25%(!!!).
- */
- skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
+ * Truesize is the actual allocation size, even if the
+ * allocation is only partially used.
+ */
+ skb->truesize += PAGE_SIZE * skb_shinfo(skb)->nr_frags;
skb->len += skb->data_len;
if (rx->flags & XEN_NETRXF_csum_blank)
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Neil Horman @ 2013-01-07 15:38 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David Miller, Linus Torvalds, Vlad Yasevich, linux-sctp
In-Reply-To: <50EAE68C.2050300@openwrt.org>
On Mon, Jan 07, 2013 at 04:15:24PM +0100, Florian Fainelli wrote:
> Le 01/07/13 15:49, Neil Horman a écrit :
> >On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
> >>Hello Neil,
> >>
> >>Le 12/15/12 02:22, Neil Horman a écrit :
> >>>Recently I posted commit 3c68198e75 which made selection of the cookie hmac
> >>>algorithm selectable. This is all well and good, but Linus noted that it
> >>>changes the default config:
> >>>http://marc.info/?l=linux-netdev&m=135536629004808&w=2
> >>>
> >>>I've modified the sctp Kconfig file to reflect the recommended way of making
> >>>this choice, using the thermal driver example specified, and brought the
> >>>defaults back into line with the way they were prior to my origional patch
> >>>
> >>>Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
> >>>algorithm, so we don't needlessly bloat the kernel by forcing a non-none
> >>>default. This also led me to note that we won't honor the default none
> >>>condition properly because of how sctp_net_init is encoded. Fix that up as
> >>>well.
> >>>
> >>>Tested by myself (allbeit fairly quickly). All configuration combinations seems
> >>>to work soundly.
> >>>
> >>>Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >>>CC: David Miller <davem@davemloft.net>
> >>>CC: Linus Torvalds <torvalds@linux-foundation.org>
> >>>CC: Vlad Yasevich <vyasevich@gmail.com>
> >>>CC: linux-sctp@vger.kernel.org
> >>>---
> >>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
> >>> net/sctp/protocol.c | 4 ++--
> >>> 2 files changed, 27 insertions(+), 4 deletions(-)
> >>>
> >>>diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
> >>>index a9edd2e..c262106 100644
> >>>--- a/net/sctp/Kconfig
> >>>+++ b/net/sctp/Kconfig
> >>>@@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
> >>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
> >>>
> >>> If unsure, say N
> >>>+choice
> >>>+ prompt "Default SCTP cookie HMAC encoding"
> >>>+ default SCTP_COOKIE_HMAC_MD5
> >>Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
> >>update to 3.8-rc2, and I usually build my kernel-headers with:
> >>
> >>yes '' | ARCH=foo make oldconfig
> >>
> >>and this just kept asking me for this config symbol because none
> >>could be provided.
> >>--
> >>Florian
> >>
> >No, the config mechanism is setup to offer the user the ability to choose a
> >default cookie hmac, alg, then optionally select any other hmac algs you would
> >like to be made available (in the event you want to change the default at run
> >time). When you select the default, it eables (via the select directive), the
> >corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
> >then prompts for the remaining values.
>
> Ok for the explanation, but this still breaks an oldconfig because
> we do not actually propose the user with a default choice:
>
> choice[1-3?]: Default SCTP cookie HMAC encoding
> 1. Enable optional MD5 hmac cookie generation
> (SCTP_DEFAULT_COOKIE_HMAC_MD5) (NEW)
> 2. Enable optional SHA1 hmac cookie generation
> (SCTP_DEFAULT_COOKIE_HMAC_SHA1) (NEW)
> 3. Use no hmac alg in SCTP cookie generation
> (SCTP_DEFAULT_COOKIE_HMAC_NONE) (NEW)
>
> I do not see any difference in what I am proposed if the default
> config symbol is SCTP_DEFAULT_COOKIE_HMAC_MD5, I can still
> optionally choose SHA1 to be supported, and I do have a valid
> default config for this choice. While if I keep SCTP_COOKIE_HMAC_MD5
No, thats the problem, your old config is no longer valid with this new Kconfig
file. Your config is telling the config utility that you want your default
Cookie hmac to be MD5, but you've explicitly told it (via your yes "" | make
oldconfig command), that you want SCTP_COOKIE_HMAC_MD5 to be disabled, so the
config utility is left with no choice to prompt you again for a default hmac,
which your command answers again by saying SCTP_DEFAULT_COOKIE_HMAC_MD5 (the
default choice of 1). Thats your loop, you keep telling the config utility that
you both want the default hmac to be md5, and that you don't want to allow md5
to be an available hmac alg.
Thats not a bug. I'm sorry if your old configuration needs manual updating, but
there are no guarantees that old configurations will 'just work' in perpituity.
Neil
> as the default I have to manually enter which option I want.
> --
> Florian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 1/2] ipv6: avoid blackhole and prohibited entries upon prefix purge [v2]
From: Nicolas Dichtel @ 2013-01-07 15:43 UTC (permalink / raw)
To: Romain KUNTZ; +Cc: netdev, Eric Dumazet, yoshfuji, davem
In-Reply-To: <3EF640F8-5242-486E-B7A3-9DA2A88F5A4F@ipflavors.com>
Le 07/01/2013 12:30, Romain KUNTZ a écrit :
> Hello Nicolas,
>
> On Jan 7, 2013, at 11:25 , Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
>> Le 05/01/2013 22:44, Romain KUNTZ a écrit :
>>> Mobile IPv6 provokes a kernel Oops since commit 64c6d08e (ipv6:
>>> del unreachable route when an addr is deleted on lo), because
>>> ip6_route_lookup() may also return blackhole and prohibited
>>> entry. However, these entries have a NULL rt6i_table argument,
>>> which provokes an Oops in __ip6_del_rt() when trying to lock
>>> rt6i_table->tb6_lock.
>>>
>>> Beside, when purging a prefix, blakhole and prohibited entries
>>> should not be selected because they are not what we are looking
>>> for.
>>>
>>> We fix this by adding two new lookup flags (RT6_LOOKUP_F_NO_BLK_HOLE
>>> and RT6_LOOKUP_F_NO_PROHIBIT) in order to ensure that such entries
>>> are skipped during lookup and that the correct entry is returned.
>>>
>>> [v2]: use 'goto out;' instead of 'goto again;' to avoid unnecessary
>>> oprations on rt (as suggested by Eric Dumazet).
>>>
>>> Signed-off-by: Romain Kuntz <r.kuntz@ipflavors.com>
>>> ---
>>> include/net/ip6_route.h | 2 ++
>>> net/ipv6/addrconf.c | 4 +++-
>>> net/ipv6/fib6_rules.c | 4 ++++
>>> 3 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
>>> index 27d8318..3c93743 100644
>>> --- a/include/net/ip6_route.h
>>> +++ b/include/net/ip6_route.h
>>> @@ -30,6 +30,8 @@ struct route_info {
>>> #define RT6_LOOKUP_F_SRCPREF_TMP 0x00000008
>>> #define RT6_LOOKUP_F_SRCPREF_PUBLIC 0x00000010
>>> #define RT6_LOOKUP_F_SRCPREF_COA 0x00000020
>>> +#define RT6_LOOKUP_F_NO_BLK_HOLE 0x00000040
>>> +#define RT6_LOOKUP_F_NO_PROHIBIT 0x00000080
>>>
>>> /*
>>> * rt6_srcprefs2flags() and rt6_flags2srcprefs() translate
>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>> index 408cac4a..1891e23 100644
>>> --- a/net/ipv6/addrconf.c
>>> +++ b/net/ipv6/addrconf.c
>>> @@ -948,7 +948,9 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>>> fl6.flowi6_oif = ifp->idev->dev->ifindex;
>>> fl6.daddr = prefix;
>>> rt = (struct rt6_info *)ip6_route_lookup(net, &fl6,
>>> - RT6_LOOKUP_F_IFACE);
>>> + RT6_LOOKUP_F_IFACE |
>>> + RT6_LOOKUP_F_NO_BLK_HOLE |
>>> + RT6_LOOKUP_F_NO_PROHIBIT);
>>>
>>> if (rt != net->ipv6.ip6_null_entry &&
>> Is it not simpler to test the result here (net->ipv6.ip6_blk_hole_entry and
>> net->ipv6.ip6_prohibit_entry) like for the null_entry?
>> It will also avoid adding more flags.
>
> Your proposal would only solve part of the problem (the Oops in __ip6_del_rt()). Another problem here is that blackhole and prohibited rules should not be selected when trying to purge a prefix (correct me if I'm wrong) because they are not what we are looking for. This can prevent the targeted prefix from being purged.
In fact, I'm not sure to get the scenario. This part of the code just tries
to remove the connected prefix, added by the kernel when the address was added.
Can you describe your scenario?
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Neil Horman @ 2013-01-07 15:46 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <50EAEA71.1060007@gmail.com>
On Mon, Jan 07, 2013 at 10:32:01AM -0500, Vlad Yasevich wrote:
> On 01/07/2013 09:49 AM, Neil Horman wrote:
> >On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
> >>Hello Neil,
> >>
> >>Le 12/15/12 02:22, Neil Horman a écrit :
> >>>Recently I posted commit 3c68198e75 which made selection of the cookie hmac
> >>>algorithm selectable. This is all well and good, but Linus noted that it
> >>>changes the default config:
> >>>http://marc.info/?l=linux-netdev&m=135536629004808&w=2
> >>>
> >>>I've modified the sctp Kconfig file to reflect the recommended way of making
> >>>this choice, using the thermal driver example specified, and brought the
> >>>defaults back into line with the way they were prior to my origional patch
> >>>
> >>>Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
> >>>algorithm, so we don't needlessly bloat the kernel by forcing a non-none
> >>>default. This also led me to note that we won't honor the default none
> >>>condition properly because of how sctp_net_init is encoded. Fix that up as
> >>>well.
> >>>
> >>>Tested by myself (allbeit fairly quickly). All configuration combinations seems
> >>>to work soundly.
> >>>
> >>>Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >>>CC: David Miller <davem@davemloft.net>
> >>>CC: Linus Torvalds <torvalds@linux-foundation.org>
> >>>CC: Vlad Yasevich <vyasevich@gmail.com>
> >>>CC: linux-sctp@vger.kernel.org
> >>>---
> >>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
> >>> net/sctp/protocol.c | 4 ++--
> >>> 2 files changed, 27 insertions(+), 4 deletions(-)
> >>>
> >>>diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
> >>>index a9edd2e..c262106 100644
> >>>--- a/net/sctp/Kconfig
> >>>+++ b/net/sctp/Kconfig
> >>>@@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
> >>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
> >>>
> >>> If unsure, say N
> >>>+choice
> >>>+ prompt "Default SCTP cookie HMAC encoding"
> >>>+ default SCTP_COOKIE_HMAC_MD5
> >>
> >>Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
> >>update to 3.8-rc2, and I usually build my kernel-headers with:
> >>
> >>yes '' | ARCH=foo make oldconfig
> >>
> >>and this just kept asking me for this config symbol because none
> >>could be provided.
> >>--
> >>Florian
> >>
> >
> >No, the config mechanism is setup to offer the user the ability to choose a
> >default cookie hmac, alg, then optionally select any other hmac algs you would
> >like to be made available (in the event you want to change the default at run
> >time). When you select the default, it eables (via the select directive), the
> >corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
> >then prompts for the remaining values.
> >
>
> Neil
>
> Actually, I think it should be as Florian suggests. The default
> value of the choice should be one of the values defined as part of
> the choice (the SCTP_DEFAULT_COOKIE_*). Turning on appropriate
> default would turn on appropriate cookie config
> (SCTP_COOKIE_HMAC_*).
>
I absolutely disagree.
> Would that save all the config trouble?
>
Yes, it would fix it as Florian has noted, but at the cost of silently modifying
what the default hmac config vaule is. If you've expressly disabled
SCTP_COOKIE_HMAC_MD5, and then blindly take the default choice in the
SCTP_DEFAULT_COOKIE selection option (the default default as it were), using the
approach your suggesting, then that will silently enable SCTP_COOKIE_HMAC_MD5
again, which may not be expected by users. If you expressly have a config
option disabled in an old configuration, we should leave it there.
We're doing the right thing now, IMO. When presented with a conflictly set
of configuration options, the config utilty is (repeatedly) prompting us to
resolve them. That seems like a much more reasonable approach to this, than
silently changing pre-existing options so people can do the equivalent of just
blindly pressing enter through the config process (which is all yes "" | make
oldconfig is).
This is a momentary hiccup, corrected by taking 30 seconds to make a manual
config change (or by taking a second to understand what the config utility is
tell us by prompting for a default choice repeatedly). Theres nothing to fix
here.
Neil
> -vlad
>
> >Neil
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-07 15:48 UTC (permalink / raw)
To: Neil Horman
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <20130107153812.GC31577@hmsreliant.think-freely.org>
On 01/07/2013 10:38 AM, Neil Horman wrote:
> On Mon, Jan 07, 2013 at 04:15:24PM +0100, Florian Fainelli wrote:
>> Le 01/07/13 15:49, Neil Horman a écrit :
>>> On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
>>>> Hello Neil,
>>>>
>>>> Le 12/15/12 02:22, Neil Horman a écrit :
>>>>> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
>>>>> algorithm selectable. This is all well and good, but Linus noted that it
>>>>> changes the default config:
>>>>> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>>>>>
>>>>> I've modified the sctp Kconfig file to reflect the recommended way of making
>>>>> this choice, using the thermal driver example specified, and brought the
>>>>> defaults back into line with the way they were prior to my origional patch
>>>>>
>>>>> Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
>>>>> algorithm, so we don't needlessly bloat the kernel by forcing a non-none
>>>>> default. This also led me to note that we won't honor the default none
>>>>> condition properly because of how sctp_net_init is encoded. Fix that up as
>>>>> well.
>>>>>
>>>>> Tested by myself (allbeit fairly quickly). All configuration combinations seems
>>>>> to work soundly.
>>>>>
>>>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>>>> CC: David Miller <davem@davemloft.net>
>>>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>>>> CC: linux-sctp@vger.kernel.org
>>>>> ---
>>>>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
>>>>> net/sctp/protocol.c | 4 ++--
>>>>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>>>> index a9edd2e..c262106 100644
>>>>> --- a/net/sctp/Kconfig
>>>>> +++ b/net/sctp/Kconfig
>>>>> @@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
>>>>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
>>>>>
>>>>> If unsure, say N
>>>>> +choice
>>>>> + prompt "Default SCTP cookie HMAC encoding"
>>>>> + default SCTP_COOKIE_HMAC_MD5
>>>> Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
>>>> update to 3.8-rc2, and I usually build my kernel-headers with:
>>>>
>>>> yes '' | ARCH=foo make oldconfig
>>>>
>>>> and this just kept asking me for this config symbol because none
>>>> could be provided.
>>>> --
>>>> Florian
>>>>
>>> No, the config mechanism is setup to offer the user the ability to choose a
>>> default cookie hmac, alg, then optionally select any other hmac algs you would
>>> like to be made available (in the event you want to change the default at run
>>> time). When you select the default, it eables (via the select directive), the
>>> corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
>>> then prompts for the remaining values.
>>
>> Ok for the explanation, but this still breaks an oldconfig because
>> we do not actually propose the user with a default choice:
>>
>> choice[1-3?]: Default SCTP cookie HMAC encoding
>> 1. Enable optional MD5 hmac cookie generation
>> (SCTP_DEFAULT_COOKIE_HMAC_MD5) (NEW)
>> 2. Enable optional SHA1 hmac cookie generation
>> (SCTP_DEFAULT_COOKIE_HMAC_SHA1) (NEW)
>> 3. Use no hmac alg in SCTP cookie generation
>> (SCTP_DEFAULT_COOKIE_HMAC_NONE) (NEW)
>>
>> I do not see any difference in what I am proposed if the default
>> config symbol is SCTP_DEFAULT_COOKIE_HMAC_MD5, I can still
>> optionally choose SHA1 to be supported, and I do have a valid
>> default config for this choice. While if I keep SCTP_COOKIE_HMAC_MD5
> No, thats the problem, your old config is no longer valid with this new Kconfig
> file. Your config is telling the config utility that you want your default
> Cookie hmac to be MD5, but you've explicitly told it (via your yes "" | make
> oldconfig command), that you want SCTP_COOKIE_HMAC_MD5 to be disabled, so the
> config utility is left with no choice to prompt you again for a default hmac,
> which your command answers again by saying SCTP_DEFAULT_COOKIE_HMAC_MD5 (the
> default choice of 1). Thats your loop, you keep telling the config utility that
> you both want the default hmac to be md5, and that you don't want to allow md5
> to be an available hmac alg.
>
> Thats not a bug. I'm sorry if your old configuration needs manual updating, but
> there are no guarantees that old configurations will 'just work' in perpituity.
>
Neil
Actually, I think we have a bug in the config. Look at the thermal
driver config again. It has:
choice
prompt "Default Thermal governor"
default THERMAL_DEFAULT_GOV_STEP_WISE
config THERMAL_DEFAULT_GOV_STEP_WISE
...
config THERMAL_DEFAULT_GOV_FAIR_SHARE
...
config THERMAL_DEFAULT_GOV_USER_SPACE
...
endchoice
SCTP has:
choice
prompt "Default SCTP cookie HMAC encoding"
default SCTP_COOKIE_HMAC_MD5
config SCTP_DEFAULT_COOKIE_HMAC_MD5
...
config SCTP_DEFAULT_COOKIE_HMAC_SHA1
...
config SCTP_DEFAULT_COOKIE_HMAC_NONE
...
endchoice
See the difference? The default value of the choice statement needs to
be one of the available choices.
-vlad
> Neil
>
>> as the default I have to manually enter which option I want.
>> --
>> Florian
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [BUG] via_rhine stops making new tcp connections.
From: Stephen Hemminger @ 2013-01-07 15:55 UTC (permalink / raw)
To: Roger Luethi; +Cc: netdev
Please look at this bug report from davidwhodgins@gmail.com
https://bugzilla.kernel.org/show_bug.cgi?id=52151
As shown in the pcap file
https://bugs.mageia.org/attachment.cgi?id=3304
after a period of time ranging from hours to weeks,
the driver starts replying to syn,ack messages with reset,
instead of ack messages.
Rebooting corrects the problem, as shown by the pcap file
https://bugs.mageia.org/attachment.cgi?id=3305
^ permalink raw reply
* [RFC PATCH V2] tcp: introduce raw access to experimental options
From: elelueck @ 2013-01-07 16:13 UTC (permalink / raw)
To: netdev; +Cc: Einar Lueck, davem, ubacher, raspl, frankbla, samudrala
From: Einar Lueck <elelueck@linux.vnet.ibm.com>
V2 addresses the privilige concern. CAP_NET_RAW is now required for all of the
new socket options.
Einar Lueck (1):
tcp: introduce raw access to experimental options
include/linux/tcp.h | 23 +++++++++
include/net/tcp.h | 3 ++
include/uapi/linux/tcp.h | 3 ++
net/ipv4/tcp.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_input.c | 119 ++++++++++++++++++++++++++++++---------------
net/ipv4/tcp_ipv4.c | 14 ++++++
net/ipv4/tcp_minisocks.c | 17 +++++++
net/ipv4/tcp_output.c | 47 +++++++++++-------
8 files changed, 293 insertions(+), 55 deletions(-)
--
1.7.12.4
^ permalink raw reply
* [RFC PATCH V2] tcp: introduce raw access to experimental options
From: elelueck @ 2013-01-07 16:13 UTC (permalink / raw)
To: netdev; +Cc: Einar Lueck, davem, ubacher, raspl, frankbla, samudrala
In-Reply-To: <1357575210-52926-1-git-send-email-elelueck@linux.vnet.ibm.com>
From: Einar Lueck <elelueck@linux.vnet.ibm.com>
This patch adds means for raw acces to TCP expirimental options
253 and 254. The intention of this is to enable user space
applications to implement communication behaviour that depends
on experimental options. For that, new (set|get)sockopts are
introduced:
TCP_EXPOPTS (get & set): TCP experimental options to be added to
packets
TCP_RECV_EXPOPTS (get): experimental options received with last
packet
TCP_RECV_SYN_EXPOPTS (get): experimental options received with
SYN packet
Access to these options is allowed only with CAP_NET_RAW privilige.
TCP experimental options 253 and 254 configured via TCP_EXPOPTS on
any TCP socket are appended to every packet that is sent as long
as there is enough room left. If there is not enough room left they
are silently dropped.
Listening sockets reply to SYN packets with SYN ACK packets containing
TCP experimental options 253 and 254 as configured via TCP_EXPOPTS, too.
If a TCP connection gets established the configured experimental options
are the defaults for the new socket, too. Thus, a getsockopt on the
resulting accept socket for TCP_EXPOPTS returns the same stuff configured
on the listening socket.
As mentioned above, even after the 3whs is complete, experimental options
are sent with every packet. To enable user space applications to distinguish
between what has been advertized via SYN and what has been received with the
last packet the aforementioned TCP_RECV_SYN_EXPOPTS and TCP_RECV_EXPOPTS are
introduced.
Today, experimental option 253 (COOKIE) and 254 (FASTOPEN) are already
exploited. For co-existence the following approach has been taken:
General remarks:
* Interface to COOKIE and FASTOPEN stays the same
Sender side:
1. COOKIE and FASTPATH code adds own options first (if applicable)
2. Finally, if enough room is left, TCP_EXPOPTS experimental options are
appended
Receiver side:
1. ALL 253 and 254 experimental options are made available via
TCP_RECV(_SYN)_EXPOPTS
2. COOKIE and FASTOPEN code check if there is any option relevant for them
References:
http://tools.ietf.org/html/draft-ietf-tcpm-experimental-options-02
Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
---
include/linux/tcp.h | 23 +++++++++
include/net/tcp.h | 3 ++
include/uapi/linux/tcp.h | 3 ++
net/ipv4/tcp.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_input.c | 119 ++++++++++++++++++++++++++++++---------------
net/ipv4/tcp_ipv4.c | 14 ++++++
net/ipv4/tcp_minisocks.c | 17 +++++++
net/ipv4/tcp_output.c | 47 +++++++++++-------
8 files changed, 293 insertions(+), 55 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 60b7aac..574c26f 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -67,6 +67,8 @@ struct tcp_sack_block {
#define TCP_FACK_ENABLED (1 << 1) /*1 = FACK is enabled locally*/
#define TCP_DSACK_SEEN (1 << 2) /*1 = DSACK was received from peer*/
+#define TCP_EXPOP_MAXLEN 40
+
struct tcp_options_received {
/* PAWS/RTTM data */
long ts_recent_stamp;/* Time we stored ts_recent (for aging) */
@@ -86,6 +88,9 @@ struct tcp_options_received {
u8 num_sacks; /* Number of SACK blocks */
u16 user_mss; /* mss requested by user in ioctl */
u16 mss_clamp; /* Maximal mss, negotiated at connection setup */
+ u8 exp_opts_len; /* length of buffer containing all exp
+ * options in format: kind length data */
+ u8 exp_opts[TCP_EXPOP_MAXLEN]; /* experimental options */
};
static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
@@ -93,6 +98,7 @@ static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
rx_opt->tstamp_ok = rx_opt->sack_ok = 0;
rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
rx_opt->cookie_plus = 0;
+ rx_opt->exp_opts_len = 0;
}
/* This is the max number of SACKS that we'll generate and process. It's safe
@@ -118,6 +124,11 @@ struct tcp_request_sock {
* FastOpen it's the seq#
* after data-in-SYN.
*/
+ u8 syn_expopts[TCP_EXPOP_MAXLEN];
+ /* experimental options
+ * received with SYNACK */
+ u8 syn_expopts_len;
+
};
static inline struct tcp_request_sock *tcp_rsk(const struct request_sock *req)
@@ -210,6 +221,18 @@ struct tcp_sock {
u32 snd_up; /* Urgent pointer */
u8 keepalive_probes; /* num of allowed keep alive probes */
+
+/* for raw acces to experimental options */
+ struct {
+ u8 *conf; /* lazy allocation of TCP_EXPOP_MAXLEN bytes
+ * for raw access to experimental options */
+ u8 conf_len; /* bytes actually used for experimental opts */
+ u8 *syn; /* experimental options received with SYN,
+ * allocated only if received */
+ u8 syn_len; /* bytes of experimental options actually
+ * received with SYN */
+ } exp_opts;
+
/*
* Options received (usually on last packet, some only on SYN packets).
*/
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4af45e3..8e0740c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -180,6 +180,8 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
#define TCPOPT_TIMESTAMP 8 /* Better RTT estimations/PAWS */
#define TCPOPT_MD5SIG 19 /* MD5 Signature (RFC2385) */
#define TCPOPT_COOKIE 253 /* Cookie extension (experimental) */
+#define TCPOPT_EXP253 253 /* TCP experimental option 253 */
+#define TCPOPT_EXP254 254 /* TCP experimental option 254 */
#define TCPOPT_EXP 254 /* Experimental */
/* Magic number to be after the option value for sharing TCP
* experimental options. See draft-ietf-tcpm-experimental-options-00.txt
@@ -190,6 +192,7 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
* TCP option lengths
*/
+#define TCPOLEN_MAX_ANYEXP 40
#define TCPOLEN_MSS 4
#define TCPOLEN_WINDOW 3
#define TCPOLEN_SACK_PERM 2
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index e962faa..a6554d1 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -111,6 +111,9 @@ enum {
#define TCP_QUEUE_SEQ 21
#define TCP_REPAIR_OPTIONS 22
#define TCP_FASTOPEN 23 /* Enable FastOpen on listeners */
+#define TCP_EXPOPTS 24 /* TCP exp. options (configured) */
+#define TCP_RECV_EXPOPTS 25 /* TCP exp. options (received) */
+#define TCP_RECV_SYN_EXPOPTS 26 /* TCP exp. options (rec. with syn)) */
struct tcp_repair_opt {
__u32 opt_code;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e457c7a..437c22e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -423,6 +423,12 @@ void tcp_init_sock(struct sock *sk)
sk->sk_sndbuf = sysctl_tcp_wmem[1];
sk->sk_rcvbuf = sysctl_tcp_rmem[1];
+ /* memory for raw access to experimental options is allocated lazy */
+ tp->exp_opts.conf = NULL;
+ tp->exp_opts.conf_len = 0;
+ tp->exp_opts.syn = NULL;
+ tp->exp_opts.syn_len = 0;
+
local_bh_disable();
sock_update_memcg(sk);
sk_sockets_allocated_inc(sk);
@@ -2366,6 +2372,56 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
/* These are data/string values, all the others are ints */
switch (optname) {
+ case TCP_EXPOPTS: {
+ u8 conf[TCP_EXPOP_MAXLEN];
+
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (optlen > TCP_EXPOP_MAXLEN || (optlen < 4 && optlen > 0) ||
+ (optlen % 4 > 0))
+ return -EINVAL;
+ if (optlen > 0 && !optval)
+ return -EINVAL;
+
+ /* filter for raw access to supported options */
+ if (optlen) {
+ u8 i;
+
+ if (copy_from_user(conf, optval, optlen))
+ return -EFAULT;
+
+ i = 0;
+ while (i < optlen) {
+ if (conf[i] != TCPOPT_EXP253 &&
+ conf[i] != TCPOPT_EXP254)
+ return -EINVAL;
+
+ if (i + 1 < optlen) {
+ i += conf[i+1];
+ if (i > optlen)
+ return -EINVAL;
+ } else {
+ return -EINVAL;
+ }
+ }
+ }
+
+ lock_sock(sk);
+ if (!optlen) {
+ tp->exp_opts.conf_len = 0;
+ release_sock(sk);
+ return 0;
+ }
+ if (!tp->exp_opts.conf) {
+ tp->exp_opts.conf = kzalloc(TCP_EXPOP_MAXLEN,
+ sk->sk_allocation);
+ }
+ memcpy(tp->exp_opts.conf, conf, optlen);
+ tp->exp_opts.conf_len = optlen;
+ release_sock(sk);
+ return err;
+ }
case TCP_CONGESTION: {
char name[TCP_CA_NAME_MAX];
@@ -2947,6 +3003,72 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
case TCP_USER_TIMEOUT:
val = jiffies_to_msecs(icsk->icsk_user_timeout);
break;
+ case TCP_EXPOPTS: {
+ u8 exp_opts_len;
+
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (get_user(len, optlen))
+ return -EFAULT;
+ if (len < 0)
+ return -EINVAL;
+
+ exp_opts_len = tp->exp_opts.conf_len;
+
+ if (exp_opts_len > len)
+ return -EINVAL;
+ if (put_user(exp_opts_len, optlen))
+ return -EFAULT;
+ if (exp_opts_len && copy_to_user(optval, tp->exp_opts.conf,
+ exp_opts_len))
+ return -EFAULT;
+ return 0;
+ }
+ case TCP_RECV_EXPOPTS:
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (get_user(len, optlen))
+ return -EFAULT;
+ if (len < 0)
+ return -EINVAL;
+
+ if (len < tp->rx_opt.exp_opts_len)
+ return -EINVAL;
+
+ if (put_user(tp->rx_opt.exp_opts_len, optlen))
+ return -EFAULT;
+ if (copy_to_user(optval, tp->rx_opt.exp_opts,
+ tp->rx_opt.exp_opts_len))
+ return -EFAULT;
+ return 0;
+ case TCP_RECV_SYN_EXPOPTS: {
+ u8 exp_opts_len;
+
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (get_user(len, optlen))
+ return -EFAULT;
+ if (len < 0)
+ return -EINVAL;
+
+ if (!tp->exp_opts.syn)
+ exp_opts_len = 0;
+ else
+ exp_opts_len = tp->exp_opts.syn_len;
+
+ if (exp_opts_len > len)
+ return -EINVAL;
+ if (put_user(exp_opts_len, optlen))
+ return -EFAULT;
+ if (exp_opts_len && copy_to_user(optval, tp->exp_opts.syn,
+ exp_opts_len)) {
+ return -EFAULT;
+ }
+ return 0;
+ }
default:
return -ENOPROTOOPT;
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 181fc82..ab9c88b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3699,11 +3699,32 @@ old_ack:
return 0;
}
+static inline void tcp_parse_fastopen_cookie(int opcode,
+ int opsize,
+ const unsigned char *ptr,
+ struct tcp_fastopen_cookie *foc,
+ const struct tcphdr *th) {
+ /* Fast Open option shares code 254 using a 16 bits magic number. It's
+ * valid only in SYN or SYN-ACK with an even size.
+ */
+ if (opsize < TCPOLEN_EXP_FASTOPEN_BASE ||
+ get_unaligned_be16(ptr) != TCPOPT_FASTOPEN_MAGIC || foc == NULL ||
+ !th->syn || (opsize & 1))
+ return;
+ foc->len = opsize - TCPOLEN_EXP_FASTOPEN_BASE;
+ if (foc->len >= TCP_FASTOPEN_COOKIE_MIN &&
+ foc->len <= TCP_FASTOPEN_COOKIE_MAX)
+ memcpy(foc->val, ptr + 2, foc->len);
+ else if (foc->len != 0)
+ foc->len = -1;
+}
+
/* Look for tcp options. Normally only called on SYN and SYNACK packets.
* But, this can also be called on packets in the established flow when
* the fast version below fails.
*/
-void tcp_parse_options(const struct sk_buff *skb, struct tcp_options_received *opt_rx,
+void tcp_parse_options(const struct sk_buff *skb,
+ struct tcp_options_received *opt_rx,
const u8 **hvpp, int estab,
struct tcp_fastopen_cookie *foc)
{
@@ -3713,6 +3734,7 @@ void tcp_parse_options(const struct sk_buff *skb, struct tcp_options_received *o
ptr = (const unsigned char *)(th + 1);
opt_rx->saw_tstamp = 0;
+ opt_rx->exp_opts_len = 0;
while (length > 0) {
int opcode = *ptr++;
@@ -3788,48 +3810,56 @@ void tcp_parse_options(const struct sk_buff *skb, struct tcp_options_received *o
*/
break;
#endif
- case TCPOPT_COOKIE:
- /* This option is variable length.
+ case TCPOPT_EXP253:
+ case TCPOPT_EXP254:
+ /* First parse options into raw access area for
+ * experimental options. Then handle
+ * potential exploitations
*/
- switch (opsize) {
- case TCPOLEN_COOKIE_BASE:
- /* not yet implemented */
- break;
- case TCPOLEN_COOKIE_PAIR:
- /* not yet implemented */
- break;
- case TCPOLEN_COOKIE_MIN+0:
- case TCPOLEN_COOKIE_MIN+2:
- case TCPOLEN_COOKIE_MIN+4:
- case TCPOLEN_COOKIE_MIN+6:
- case TCPOLEN_COOKIE_MAX:
- /* 16-bit multiple */
- opt_rx->cookie_plus = opsize;
- *hvpp = ptr;
- break;
- default:
- /* ignore option */
- break;
+ if (opsize <= TCPOLEN_MAX_ANYEXP &&
+ opsize >= 2 &&
+ (opt_rx->exp_opts_len + opsize <=
+ TCPOLEN_MAX_ANYEXP)) {
+ opt_rx->exp_opts[
+ opt_rx->exp_opts_len] = opcode;
+ opt_rx->exp_opts[
+ opt_rx->exp_opts_len + 1] =
+ opsize;
+ memcpy(opt_rx->exp_opts +
+ opt_rx->exp_opts_len + 2, ptr,
+ opsize - 2);
+ opt_rx->exp_opts_len += opsize;
}
- break;
- case TCPOPT_EXP:
- /* Fast Open option shares code 254 using a
- * 16 bits magic number. It's valid only in
- * SYN or SYN-ACK with an even size.
- */
- if (opsize < TCPOLEN_EXP_FASTOPEN_BASE ||
- get_unaligned_be16(ptr) != TCPOPT_FASTOPEN_MAGIC ||
- foc == NULL || !th->syn || (opsize & 1))
- break;
- foc->len = opsize - TCPOLEN_EXP_FASTOPEN_BASE;
- if (foc->len >= TCP_FASTOPEN_COOKIE_MIN &&
- foc->len <= TCP_FASTOPEN_COOKIE_MAX)
- memcpy(foc->val, ptr + 2, foc->len);
- else if (foc->len != 0)
- foc->len = -1;
+ /* handle potential exploitations */
+ if (opcode == TCPOPT_COOKIE) {
+ /* This option is variable length. */
+ switch (opsize) {
+ case TCPOLEN_COOKIE_BASE:
+ /* not yet implemented */
+ break;
+ case TCPOLEN_COOKIE_PAIR:
+ /* not yet implemented */
+ break;
+ case TCPOLEN_COOKIE_MIN+0:
+ case TCPOLEN_COOKIE_MIN+2:
+ case TCPOLEN_COOKIE_MIN+4:
+ case TCPOLEN_COOKIE_MIN+6:
+ case TCPOLEN_COOKIE_MAX:
+ /* 16-bit multiple */
+ opt_rx->cookie_plus = opsize;
+ *hvpp = ptr;
+ break;
+ default:
+ /* ignore option */
+ break;
+ }
+ } else {
+ tcp_parse_fastopen_cookie(opcode,
+ opsize, ptr,
+ foc, th);
+ }
break;
-
}
ptr += opsize-2;
length -= opsize;
@@ -3861,6 +3891,9 @@ static bool tcp_fast_parse_options(const struct sk_buff *skb,
const struct tcphdr *th,
struct tcp_sock *tp, const u8 **hvpp)
{
+ /* required if exp options are not used anymore by the counter part */
+ tp->rx_opt.exp_opts_len = 0;
+
/* In the spirit of fast parsing, compare doff directly to constant
* values. Because equality is used, short doff can be ignored here.
*/
@@ -5786,6 +5819,14 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
}
}
+ if (unlikely(tp->rx_opt.exp_opts_len > 0)) {
+ tp->exp_opts.syn = kzalloc(tp->rx_opt.exp_opts_len,
+ sk->sk_allocation);
+ tp->exp_opts.syn_len = tp->rx_opt.exp_opts_len;
+ memcpy(tp->exp_opts.syn, &tp->rx_opt.exp_opts,
+ tp->rx_opt.exp_opts_len);
+ }
+
smp_mb();
tcp_finish_connect(sk, skb);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0c4a643..b7ab768 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1525,6 +1525,16 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
tcp_parse_options(skb, &tmp_opt, &hash_location, 0,
want_cookie ? NULL : &foc);
+ /* for raw access to experimental options in SYN packet */
+ tcp_rsk(req)->syn_expopts_len = tmp_opt.exp_opts_len;
+ if (tcp_rsk(req)->syn_expopts_len) {
+ /* transport experimental options via request socket to big
+ * socket
+ */
+ memcpy(tcp_rsk(req)->syn_expopts, tmp_opt.exp_opts,
+ tcp_rsk(req)->syn_expopts_len);
+ }
+
if (tmp_opt.cookie_plus > 0 &&
tmp_opt.saw_tstamp &&
!tp->rx_opt.cookie_out_never &&
@@ -2209,6 +2219,10 @@ void tcp_v4_destroy_sock(struct sock *sk)
}
BUG_ON(tp->fastopen_rsk != NULL);
+ /* buffers for raw access to experimental options */
+ kfree(tp->exp_opts.conf);
+ kfree(tp->exp_opts.syn);
+
/* If socket is aborted during connect operation */
tcp_free_fastopen_req(tp);
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index a7302d9..1ca8980 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -468,6 +468,23 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
newtp->urg_data = 0;
+ if (tcp_rsk(req)->syn_expopts_len) {
+ newtp->exp_opts.syn_len =
+ tcp_rsk(req)->syn_expopts_len;
+ newtp->exp_opts.syn = kzalloc(newtp->exp_opts.syn_len,
+ GFP_ATOMIC);
+ memcpy(newtp->exp_opts.syn, tcp_rsk(req)->syn_expopts,
+ newtp->exp_opts.syn_len);
+ }
+
+ if (oldtp->exp_opts.conf_len > 0) {
+ newtp->exp_opts.conf_len = oldtp->exp_opts.conf_len;
+ newtp->exp_opts.conf = kzalloc(TCP_EXPOP_MAXLEN,
+ GFP_ATOMIC);
+ memcpy(newtp->exp_opts.conf, oldtp->exp_opts.conf,
+ oldtp->exp_opts.conf_len);
+ }
+
if (sock_flag(newsk, SOCK_KEEPOPEN))
inet_csk_reset_keepalive_timer(newsk,
keepalive_time_when(newtp));
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 948ac27..a57470a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -385,6 +385,7 @@ static inline bool tcp_urg_mode(const struct tcp_sock *tp)
#define OPTION_MD5 (1 << 2)
#define OPTION_WSCALE (1 << 3)
#define OPTION_COOKIE_EXTENSION (1 << 4)
+#define OPTION_EXP (1 << 5)
#define OPTION_FAST_OPEN_COOKIE (1 << 8)
struct tcp_out_options {
@@ -581,6 +582,12 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp,
}
ptr += (foc->len + 3) >> 2;
}
+ if (unlikely(OPTION_EXP & options && tp->exp_opts.conf_len > 0)) {
+ __u8 *p = (__u8 *) ptr;
+ memcpy(ptr, tp->exp_opts.conf, tp->exp_opts.conf_len);
+ p += tp->exp_opts.conf_len;
+ ptr = (__be32 *) p;
+ }
}
/* Compute TCP options for SYN packets. This is not the final
@@ -693,6 +700,11 @@ static unsigned int tcp_syn_options(struct sock *sk, struct sk_buff *skb,
remaining -= need;
}
}
+ if (unlikely(tp->exp_opts.conf_len > 0 &&
+ tp->exp_opts.conf_len <= remaining)) {
+ opts->options |= OPTION_EXP;
+ remaining -= tp->exp_opts.conf_len;
+ }
return MAX_TCP_OPTION_SPACE - remaining;
}
@@ -748,15 +760,6 @@ static unsigned int tcp_synack_options(struct sock *sk,
if (unlikely(!ireq->tstamp_ok))
remaining -= TCPOLEN_SACKPERM_ALIGNED;
}
- if (foc != NULL) {
- u32 need = TCPOLEN_EXP_FASTOPEN_BASE + foc->len;
- need = (need + 3) & ~3U; /* Align to 32 bits */
- if (remaining >= need) {
- opts->options |= OPTION_FAST_OPEN_COOKIE;
- opts->fastopen_cookie = foc;
- remaining -= need;
- }
- }
/* Similar rationale to tcp_syn_options() applies here, too.
* If the <SYN> options fit, the same options should fit now!
*/
@@ -779,6 +782,12 @@ static unsigned int tcp_synack_options(struct sock *sk,
opts->hash_size = 0;
}
}
+
+ if (unlikely(tcp_sk(sk)->exp_opts.conf_len > 0 &&
+ tcp_sk(sk)->exp_opts.conf_len <= remaining)) {
+ opts->options |= OPTION_EXP;
+ remaining -= tcp_sk(sk)->exp_opts.conf_len;
+ }
return MAX_TCP_OPTION_SPACE - remaining;
}
@@ -791,38 +800,44 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
{
struct tcp_skb_cb *tcb = skb ? TCP_SKB_CB(skb) : NULL;
struct tcp_sock *tp = tcp_sk(sk);
- unsigned int size = 0;
+ unsigned remaining = MAX_TCP_OPTION_SPACE;
unsigned int eff_sacks;
#ifdef CONFIG_TCP_MD5SIG
*md5 = tp->af_specific->md5_lookup(sk, sk);
if (unlikely(*md5)) {
opts->options |= OPTION_MD5;
- size += TCPOLEN_MD5SIG_ALIGNED;
+ remaining -= TCPOLEN_MD5SIG_ALIGNED;
}
#else
*md5 = NULL;
#endif
- if (likely(tp->rx_opt.tstamp_ok)) {
+ if (likely(tp->rx_opt.tstamp_ok &&
+ remaining >= TCPOLEN_TSTAMP_ALIGNED)) {
opts->options |= OPTION_TS;
opts->tsval = tcb ? tcb->when : 0;
opts->tsecr = tp->rx_opt.ts_recent;
- size += TCPOLEN_TSTAMP_ALIGNED;
+ remaining -= TCPOLEN_TSTAMP_ALIGNED;
}
eff_sacks = tp->rx_opt.num_sacks + tp->rx_opt.dsack;
if (unlikely(eff_sacks)) {
- const unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
opts->num_sack_blocks =
min_t(unsigned int, eff_sacks,
(remaining - TCPOLEN_SACK_BASE_ALIGNED) /
TCPOLEN_SACK_PERBLOCK);
- size += TCPOLEN_SACK_BASE_ALIGNED +
+ remaining -= TCPOLEN_SACK_BASE_ALIGNED +
opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK;
}
- return size;
+ if (unlikely(tp->exp_opts.conf_len > 0 &&
+ tp->exp_opts.conf_len <= remaining)) {
+ opts->options |= OPTION_EXP;
+ remaining -= tp->exp_opts.conf_len;
+ }
+
+ return MAX_TCP_OPTION_SPACE - remaining;
}
--
1.7.12.4
^ permalink raw reply related
* Re: [PATCH] xen/netfront: improve truesize tracking
From: Eric Dumazet @ 2013-01-07 16:17 UTC (permalink / raw)
To: Ian Campbell
Cc: Sander Eikelenboom, Rick Jones, netdev@vger.kernel.org,
Konrad Rzeszutek Wilk, annie li, xen-devel@lists.xensource.com
In-Reply-To: <1357566063.7989.68.camel@zakaz.uk.xensource.com>
On Mon, 2013-01-07 at 13:41 +0000, Ian Campbell wrote:
> > UDP UNIDIRECTIONAL SEND TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.1.1 (192.168.1.1) port 0 AF_INET : +/-2.500% @ 95% conf. : demo
> > Socket Message Elapsed Messages
> > Size Size Time Okay Errors Throughput
> > bytes bytes secs # # KBytes/sec
> >
> > current 212992 65507 60.00 252586 0 269305.73
> > current 2280 60.00 229371 244553.96
> > patched 212992 65507 60.00 256209 0 273168.32
> > patched 2280 60.00 201670 215019.54
>
> The recv numbers here aren't too pleasing either.
The number of packets that can be queued into UDP socket depends on
sk->sk_rcvbuf (SO_RCVBUF) and skb truesize.
So what we notice here are packet drops (netstat -s would give us the
total counters)
To absorb a burst of incoming messages, an application would have to set
an appropriate receive buffer.
In this case, RCVBUF value was set to a very minimum, basically not
allowing more than one queued packet.
TCP stack has a 'collapse' mode, which basically converts skbs in
receive queue (or ofo queue) to better filled ones (skb->len very close
to skb->truesize) when memory limits are about to be hit.
Its very expensive, as it adds one more copy stage, but it happens only
in rare circumstances. Of course, when a driver uses one page of 4096
bytes to store one 1514 bytes ethernet frame, it can happen more often.
netstat -s | grep collap
25292 packets collapsed in receive queue due to low socket buffer
^ permalink raw reply
* Re: [patch net-next] ethtool: consolidate work with ethtool_ops
From: Ben Hutchings @ 2013-01-07 16:19 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, richardcochran, greearb, jeffrey.t.kirsher
In-Reply-To: <1357561574-14255-1-git-send-email-jiri@resnulli.us>
On Mon, 2013-01-07 at 13:26 +0100, Jiri Pirko wrote:
> No need to check if ethtool_ops == NULL since it can't be.
> Use local variable "ops" in functions where it is present
> instead of dev->ethtool_ops
> Introduce local variable "ops" in functions where dev->ethtool_ops is used
> many times.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> net/core/ethtool.c | 45 +++++++++++++++++++++------------------------
> 1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index a870543..08c213e 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
[...]
> @@ -1082,9 +1083,10 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
> {
> struct ethtool_value id;
> static bool busy;
> + const struct ethtool_ops *ops = dev->ethtool_ops;
> int rc;
>
> - if (!dev->ethtool_ops->set_phys_id)
> + if (ops->set_phys_id)
> return -EOPNOTSUPP;
[...]
This condition is inverted.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH V2] xen/netfront: improve truesize tracking
From: Eric Dumazet @ 2013-01-07 16:19 UTC (permalink / raw)
To: Ian Campbell
Cc: netdev, Sander Eikelenboom, Konrad Rzeszutek Wilk, annie li,
xen-devel
In-Reply-To: <1357572726-7059-1-git-send-email-ian.campbell@citrix.com>
On Mon, 2013-01-07 at 15:32 +0000, Ian Campbell wrote:
> Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
> than that. We have already accounted for this in
> NETFRONT_SKB_CB(skb)->pull_to so use that instead.
>
> Fixes WARN_ON from skb_try_coalesce.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: annie li <annie.li@oracle.com>
> Cc: xen-devel@lists.xen.org
> Cc: netdev@vger.kernel.org
> Cc: stable@kernel.org # 3.7.x only
> ---
> v2:
> - acount frags as a full page as recommended by Eric.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [patch net-next] ethtool: consolidate work with ethtool_ops
From: Richard Cochran @ 2013-01-07 16:19 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, bhutchings, greearb, jeffrey.t.kirsher
In-Reply-To: <1357561574-14255-1-git-send-email-jiri@resnulli.us>
On Mon, Jan 07, 2013 at 01:26:14PM +0100, Jiri Pirko wrote:
> @@ -1346,13 +1347,9 @@ static int ethtool_get_ts_info(struct net_device *dev, void __user *useraddr)
> info.cmd = ETHTOOL_GET_TS_INFO;
>
> if (phydev && phydev->drv && phydev->drv->ts_info) {
> -
> err = phydev->drv->ts_info(phydev, &info);
> -
> - } else if (dev->ethtool_ops && dev->ethtool_ops->get_ts_info) {
> -
> + } else if (ops->get_ts_info) {
> err = ops->get_ts_info(dev, &info);
> -
> } else {
> info.so_timestamping =
> SOF_TIMESTAMPING_RX_SOFTWARE |
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply
* [PATCH 3.8-rc] net: mvneta: fix driver operation in SMP context
From: Thomas Petazzoni @ 2013-01-07 16:27 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jason Cooper, Andrew Lunn, Gregory Clement,
linux-arm-kernel, Lior Amsalem, Maen Suleiman, Dmitri Epshtein
From: Dmitri Epshtein <dima@marvell.com>
In order for the driver to behave properly in a SMP context, the same
transmit queue should be used by the kernel in dev_queue_xmit() and in
the driver's mvneta_tx() function. To achieve that, the driver now
implements the ->ndo_select_txq() operation.
For now, it always returns the same transmit queue, txq_def, until the
driver is expanded to properly take advantage of the multiqueue
capabilities of the hardware.
Without this patch, the network driver crashes the kernel almost
immediately on Armada XP platforms, if the network load is at least a
little bit parallel (i.e several threads).
[Thomas Petazzoni: reword commit message]
Signed-off-by: Dmitri Epshtein <dima@marvell.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
This is 3.8-rc material.
---
drivers/net/ethernet/marvell/mvneta.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index b6025c3..af2c421 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1310,6 +1310,17 @@ static u32 mvneta_skb_tx_csum(struct mvneta_port *pp, struct sk_buff *skb)
return MVNETA_TX_L4_CSUM_NOT;
}
+static u16 mvneta_tx_policy(struct mvneta_port *pp, struct sk_buff *skb)
+{
+ return (u16)txq_def;
+}
+
+static u16 mvneta_select_txq(struct net_device *dev, struct sk_buff *skb)
+{
+ struct mvneta_port *pp = netdev_priv(dev);
+ return mvneta_tx_policy(pp, skb);
+}
+
/* Returns rx queue pointer (find last set bit) according to causeRxTx
* value
*/
@@ -1476,7 +1487,8 @@ error:
static int mvneta_tx(struct sk_buff *skb, struct net_device *dev)
{
struct mvneta_port *pp = netdev_priv(dev);
- struct mvneta_tx_queue *txq = &pp->txqs[txq_def];
+ u16 txq_id = mvneta_tx_policy(pp, skb);
+ struct mvneta_tx_queue *txq = &pp->txqs[txq_id];
struct mvneta_tx_desc *tx_desc;
struct netdev_queue *nq;
int frags = 0;
@@ -1486,7 +1498,7 @@ static int mvneta_tx(struct sk_buff *skb, struct net_device *dev)
goto out;
frags = skb_shinfo(skb)->nr_frags + 1;
- nq = netdev_get_tx_queue(dev, txq_def);
+ nq = netdev_get_tx_queue(dev, txq_id);
/* Get a descriptor for the first part of the packet */
tx_desc = mvneta_txq_next_desc_get(txq);
@@ -2550,6 +2562,7 @@ static const struct net_device_ops mvneta_netdev_ops = {
.ndo_change_mtu = mvneta_change_mtu,
.ndo_tx_timeout = mvneta_tx_timeout,
.ndo_get_stats64 = mvneta_get_stats64,
+ .ndo_select_queue = mvneta_select_txq,
};
const struct ethtool_ops mvneta_eth_tool_ops = {
--
1.7.9.5
^ permalink raw reply related
* Re: [patch net] ethtool: set addr_assign_type to NET_ADDR_SET when addr is passed on create
From: Ben Hutchings @ 2013-01-07 16:36 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, shemminger, sassmann
In-Reply-To: <1357511921-5537-1-git-send-email-jiri@resnulli.us>
On Sun, 2013-01-06 at 23:38 +0100, Jiri Pirko wrote:
> In case user passed address via netlink during create, NET_ADDR_PERM was set.
> That is not correct so fix this by setting NET_ADDR_SET.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Not sure what this has to do with ethtool...
> ---
> net/core/rtnetlink.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 9969afb..9a419b0 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1667,9 +1667,11 @@ struct net_device *rtnl_create_link(struct net *net,
>
> if (tb[IFLA_MTU])
> dev->mtu = nla_get_u32(tb[IFLA_MTU]);
> - if (tb[IFLA_ADDRESS])
> + if (tb[IFLA_ADDRESS]) {
> memcpy(dev->dev_addr, nla_data(tb[IFLA_ADDRESS]),
> nla_len(tb[IFLA_ADDRESS]));
> + dev->addr_assign_type = NET_ADDR_SET;
> + }
> if (tb[IFLA_BROADCAST])
> memcpy(dev->broadcast, nla_data(tb[IFLA_BROADCAST]),
> nla_len(tb[IFLA_BROADCAST]));
But if the address is not specified for this new device,
addr_assign_type remains NET_ADD_PERM, right? That sort of makes sense
in that the 'permanent address' of a software device is all-zeroes. I
would prefer to have a way for devices to deny having a permanent
address, but it's a bit late to change that now.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-07 16:39 UTC (permalink / raw)
To: Neil Horman
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <20130107154625.GD31577@hmsreliant.think-freely.org>
On 01/07/2013 10:46 AM, Neil Horman wrote:
> On Mon, Jan 07, 2013 at 10:32:01AM -0500, Vlad Yasevich wrote:
>> On 01/07/2013 09:49 AM, Neil Horman wrote:
>>> On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
>>>> Hello Neil,
>>>>
>>>> Le 12/15/12 02:22, Neil Horman a écrit :
>>>>> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
>>>>> algorithm selectable. This is all well and good, but Linus noted that it
>>>>> changes the default config:
>>>>> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>>>>>
>>>>> I've modified the sctp Kconfig file to reflect the recommended way of making
>>>>> this choice, using the thermal driver example specified, and brought the
>>>>> defaults back into line with the way they were prior to my origional patch
>>>>>
>>>>> Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
>>>>> algorithm, so we don't needlessly bloat the kernel by forcing a non-none
>>>>> default. This also led me to note that we won't honor the default none
>>>>> condition properly because of how sctp_net_init is encoded. Fix that up as
>>>>> well.
>>>>>
>>>>> Tested by myself (allbeit fairly quickly). All configuration combinations seems
>>>>> to work soundly.
>>>>>
>>>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>>>> CC: David Miller <davem@davemloft.net>
>>>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>>>> CC: linux-sctp@vger.kernel.org
>>>>> ---
>>>>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
>>>>> net/sctp/protocol.c | 4 ++--
>>>>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>>>> index a9edd2e..c262106 100644
>>>>> --- a/net/sctp/Kconfig
>>>>> +++ b/net/sctp/Kconfig
>>>>> @@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
>>>>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
>>>>>
>>>>> If unsure, say N
>>>>> +choice
>>>>> + prompt "Default SCTP cookie HMAC encoding"
>>>>> + default SCTP_COOKIE_HMAC_MD5
>>>>
>>>> Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
>>>> update to 3.8-rc2, and I usually build my kernel-headers with:
>>>>
>>>> yes '' | ARCH=foo make oldconfig
>>>>
>>>> and this just kept asking me for this config symbol because none
>>>> could be provided.
>>>> --
>>>> Florian
>>>>
>>>
>>> No, the config mechanism is setup to offer the user the ability to choose a
>>> default cookie hmac, alg, then optionally select any other hmac algs you would
>>> like to be made available (in the event you want to change the default at run
>>> time). When you select the default, it eables (via the select directive), the
>>> corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
>>> then prompts for the remaining values.
>>>
>>
>> Neil
>>
>> Actually, I think it should be as Florian suggests. The default
>> value of the choice should be one of the values defined as part of
>> the choice (the SCTP_DEFAULT_COOKIE_*). Turning on appropriate
>> default would turn on appropriate cookie config
>> (SCTP_COOKIE_HMAC_*).
>>
> I absolutely disagree.
>
>> Would that save all the config trouble?
>>
> Yes, it would fix it as Florian has noted, but at the cost of silently modifying
> what the default hmac config vaule is. If you've expressly disabled
> SCTP_COOKIE_HMAC_MD5, and then blindly take the default choice in the
> SCTP_DEFAULT_COOKIE selection option (the default default as it were), using the
> approach your suggesting, then that will silently enable SCTP_COOKIE_HMAC_MD5
> again, which may not be expected by users. If you expressly have a config
> option disabled in an old configuration, we should leave it there.
GACK. Just reproduced this and I really don't like this infinite loop
of choice prompts. That's a horrible bug and we need to fix this.
I don't think overriding the value is that big of a deal, especially
considering that this is exactly what 'make menuconfig' and other
graphical configs will do.
If I start with:
CONFIG_IP_SCTP=m
CONFIG_NET_SCTPPROBE=m
# CONFIG_SCTP_DBG_MSG is not set
# CONFIG_SCTP_DBG_OBJCNT is not set
# CONFIG_SCTP_HMAC_NONE is not set
CONFIG_SCTP_HMAC_SHA1=y
# CONFIG_SCTP_HMAC_MD5 is not set
then run:
yes "" | make oldconfig
I get an infinite loop.
If I run "make menuconfig", I get:
CONFIG_IP_SCTP=m
CONFIG_NET_SCTPPROBE=m
# CONFIG_SCTP_DBG_MSG is not set
# CONFIG_SCTP_DBG_OBJCNT is not set
CONFIG_SCTP_DEFAULT_COOKIE_HMAC_MD5=y
# CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1 is not set
# CONFIG_SCTP_DEFAULT_COOKIE_HMAC_NONE is not set
CONFIG_SCTP_COOKIE_HMAC_MD5=y
# CONFIG_SCTP_COOKIE_HMAC_SHA1 is not set
Note, that SHA1 is now overridden with MD5.
If I change the value of the default choice in Kconfig, the behavior
between oldconfig and menuconfig is the same.
-vlad
>
> We're doing the right thing now, IMO. When presented with a conflictly set
> of configuration options, the config utilty is (repeatedly) prompting us to
> resolve them. That seems like a much more reasonable approach to this, than
> silently changing pre-existing options so people can do the equivalent of just
> blindly pressing enter through the config process (which is all yes "" | make
> oldconfig is).
>
> This is a momentary hiccup, corrected by taking 30 seconds to make a manual
> config change (or by taking a second to understand what the config utility is
> tell us by prompting for a default choice repeatedly). Theres nothing to fix
> here.
>
> Neil
>
>> -vlad
>>
>>> Neil
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply
* Re: [RFC PATCH V2] tcp: introduce raw access to experimental options
From: Eric Dumazet @ 2013-01-07 16:44 UTC (permalink / raw)
To: elelueck; +Cc: netdev, davem, ubacher, raspl, frankbla, samudrala
In-Reply-To: <1357575210-52926-2-git-send-email-elelueck@linux.vnet.ibm.com>
On Mon, 2013-01-07 at 17:13 +0100, elelueck@linux.vnet.ibm.com wrote:
> From: Einar Lueck <elelueck@linux.vnet.ibm.com>
>
> This patch adds means for raw acces to TCP expirimental options
> 253 and 254. The intention of this is to enable user space
> applications to implement communication behaviour that depends
> on experimental options. For that, new (set|get)sockopts are
> introduced:
>
> TCP_EXPOPTS (get & set): TCP experimental options to be added to
> packets
> TCP_RECV_EXPOPTS (get): experimental options received with last
> packet
> TCP_RECV_SYN_EXPOPTS (get): experimental options received with
> SYN packet
>
> Access to these options is allowed only with CAP_NET_RAW privilige.
>
> TCP experimental options 253 and 254 configured via TCP_EXPOPTS on
> any TCP socket are appended to every packet that is sent as long
> as there is enough room left. If there is not enough room left they
> are silently dropped.
>
> Listening sockets reply to SYN packets with SYN ACK packets containing
> TCP experimental options 253 and 254 as configured via TCP_EXPOPTS, too.
> If a TCP connection gets established the configured experimental options
> are the defaults for the new socket, too. Thus, a getsockopt on the
> resulting accept socket for TCP_EXPOPTS returns the same stuff configured
> on the listening socket.
>
> As mentioned above, even after the 3whs is complete, experimental options
> are sent with every packet. To enable user space applications to distinguish
> between what has been advertized via SYN and what has been received with the
> last packet the aforementioned TCP_RECV_SYN_EXPOPTS and TCP_RECV_EXPOPTS are
> introduced.
>
> Today, experimental option 253 (COOKIE) and 254 (FASTOPEN) are already
> exploited. For co-existence the following approach has been taken:
>
> General remarks:
> * Interface to COOKIE and FASTOPEN stays the same
> Sender side:
> 1. COOKIE and FASTPATH code adds own options first (if applicable)
> 2. Finally, if enough room is left, TCP_EXPOPTS experimental options are
> appended
> Receiver side:
> 1. ALL 253 and 254 experimental options are made available via
> TCP_RECV(_SYN)_EXPOPTS
> 2. COOKIE and FASTOPEN code check if there is any option relevant for them
>
> References:
> http://tools.ietf.org/html/draft-ietf-tcpm-experimental-options-02
>
> Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
> ---
> include/linux/tcp.h | 23 +++++++++
> include/net/tcp.h | 3 ++
> include/uapi/linux/tcp.h | 3 ++
> net/ipv4/tcp.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++
> net/ipv4/tcp_input.c | 119 ++++++++++++++++++++++++++++++---------------
> net/ipv4/tcp_ipv4.c | 14 ++++++
> net/ipv4/tcp_minisocks.c | 17 +++++++
> net/ipv4/tcp_output.c | 47 +++++++++++-------
> 8 files changed, 293 insertions(+), 55 deletions(-)
Thats a big addition, and for example doesn't help if the SYNACK should
include an option that is depending on the content of SYN message.
For TCP fastopen for example, the cookie we send to the client is not a
constant cookie.
Also TCP coalescing of TCP collapse will merge several skbs, so storing
"the last received options" in the socket is kind of not well defined
semantic.
It looks like you need to add hooks and kernel modules to fully use
experimental options, like congestion control modules.
^ permalink raw reply
* Re: [PATCH 3.8-rc] net: mvneta: fix driver operation in SMP context
From: Jason Cooper @ 2013-01-07 16:44 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, Lior Amsalem, Andrew Lunn, netdev, Maen Suleiman,
Gregory Clement, Dmitri Epshtein, linux-arm-kernel
In-Reply-To: <1357576074-24245-1-git-send-email-thomas.petazzoni@free-electrons.com>
On Mon, Jan 07, 2013 at 05:27:54PM +0100, Thomas Petazzoni wrote:
> From: Dmitri Epshtein <dima@marvell.com>
>
> In order for the driver to behave properly in a SMP context, the same
> transmit queue should be used by the kernel in dev_queue_xmit() and in
> the driver's mvneta_tx() function. To achieve that, the driver now
> implements the ->ndo_select_txq() operation.
>
> For now, it always returns the same transmit queue, txq_def, until the
> driver is expanded to properly take advantage of the multiqueue
> capabilities of the hardware.
>
> Without this patch, the network driver crashes the kernel almost
> immediately on Armada XP platforms, if the network load is at least a
> little bit parallel (i.e several threads).
>
> [Thomas Petazzoni: reword commit message]
> Signed-off-by: Dmitri Epshtein <dima@marvell.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: Jason Cooper <jason@lakedaemon.net>
I'll assume this is going through -net, let me know if it should be
otherwise.
thx,
Jason.
^ permalink raw reply
* Re: [PATCH 3.8-rc] net: mvneta: fix driver operation in SMP context
From: Ben Hutchings @ 2013-01-07 16:53 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, netdev, Jason Cooper, Andrew Lunn,
Gregory Clement, linux-arm-kernel, Lior Amsalem, Maen Suleiman,
Dmitri Epshtein
In-Reply-To: <1357576074-24245-1-git-send-email-thomas.petazzoni@free-electrons.com>
On Mon, 2013-01-07 at 17:27 +0100, Thomas Petazzoni wrote:
> From: Dmitri Epshtein <dima@marvell.com>
>
> In order for the driver to behave properly in a SMP context, the same
> transmit queue should be used by the kernel in dev_queue_xmit() and in
> the driver's mvneta_tx() function. To achieve that, the driver now
> implements the ->ndo_select_txq() operation.
>
> For now, it always returns the same transmit queue, txq_def, until the
> driver is expanded to properly take advantage of the multiqueue
> capabilities of the hardware.
[...]
Well it looks like the driver already sets up multiple TX queues, and
you just need to call skb_get_queue_mapping(skb) to look up the TX queue
in mvneta_tx().
Also since the numbers of queues are variable you should be calling
alloc_etherdev_mqs(sizeof(struct mvneta_port), txq_number, rxq_number)
instead of alloc_etherdev_mq(..., 8) in mvneta_probe().
If for some reason the current hardware initialisation doesn't actually
work for more than 1 TX queue then change the number you pass to
alloc_etherdev_mqs().
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 3.8-rc] net: mvneta: fix driver operation in SMP context
From: Gregory CLEMENT @ 2013-01-07 17:14 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, Lior Amsalem, Andrew Lunn, Jason Cooper, netdev,
Maen Suleiman, Dmitri Epshtein, linux-arm-kernel
In-Reply-To: <1357576074-24245-1-git-send-email-thomas.petazzoni@free-electrons.com>
On 01/07/2013 05:27 PM, Thomas Petazzoni wrote:
> From: Dmitri Epshtein <dima@marvell.com>
>
> In order for the driver to behave properly in a SMP context, the same
> transmit queue should be used by the kernel in dev_queue_xmit() and in
> the driver's mvneta_tx() function. To achieve that, the driver now
> implements the ->ndo_select_txq() operation.
>
> For now, it always returns the same transmit queue, txq_def, until the
> driver is expanded to properly take advantage of the multiqueue
> capabilities of the hardware.
>
> Without this patch, the network driver crashes the kernel almost
> immediately on Armada XP platforms, if the network load is at least a
> little bit parallel (i.e several threads).
>
> [Thomas Petazzoni: reword commit message]
> Signed-off-by: Dmitri Epshtein <dima@marvell.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
I have tested this patch on my Armada XP DB board and on the
OpenBlocks AX3 board. I confirmed that it fixes the problem when
multiple thread are used.
You can add my
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
and also my
Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Or a combination of the two if you prefer!
> ---
> This is 3.8-rc material.
> ---
> drivers/net/ethernet/marvell/mvneta.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index b6025c3..af2c421 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -1310,6 +1310,17 @@ static u32 mvneta_skb_tx_csum(struct mvneta_port *pp, struct sk_buff *skb)
> return MVNETA_TX_L4_CSUM_NOT;
> }
>
> +static u16 mvneta_tx_policy(struct mvneta_port *pp, struct sk_buff *skb)
> +{
> + return (u16)txq_def;
> +}
> +
> +static u16 mvneta_select_txq(struct net_device *dev, struct sk_buff *skb)
> +{
> + struct mvneta_port *pp = netdev_priv(dev);
> + return mvneta_tx_policy(pp, skb);
> +}
> +
> /* Returns rx queue pointer (find last set bit) according to causeRxTx
> * value
> */
> @@ -1476,7 +1487,8 @@ error:
> static int mvneta_tx(struct sk_buff *skb, struct net_device *dev)
> {
> struct mvneta_port *pp = netdev_priv(dev);
> - struct mvneta_tx_queue *txq = &pp->txqs[txq_def];
> + u16 txq_id = mvneta_tx_policy(pp, skb);
> + struct mvneta_tx_queue *txq = &pp->txqs[txq_id];
> struct mvneta_tx_desc *tx_desc;
> struct netdev_queue *nq;
> int frags = 0;
> @@ -1486,7 +1498,7 @@ static int mvneta_tx(struct sk_buff *skb, struct net_device *dev)
> goto out;
>
> frags = skb_shinfo(skb)->nr_frags + 1;
> - nq = netdev_get_tx_queue(dev, txq_def);
> + nq = netdev_get_tx_queue(dev, txq_id);
>
> /* Get a descriptor for the first part of the packet */
> tx_desc = mvneta_txq_next_desc_get(txq);
> @@ -2550,6 +2562,7 @@ static const struct net_device_ops mvneta_netdev_ops = {
> .ndo_change_mtu = mvneta_change_mtu,
> .ndo_tx_timeout = mvneta_tx_timeout,
> .ndo_get_stats64 = mvneta_get_stats64,
> + .ndo_select_queue = mvneta_select_txq,
> };
>
> const struct ethtool_ops mvneta_eth_tool_ops = {
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH repost] net,wireless: check against default_ethtool_ops
From: Michał Mirosław @ 2013-01-07 17:20 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: netdev, David S. Miller, Eric Dumazet, Ben Greear,
Bjørn Mork, linux-wireless, Ben Hutchings
In-Reply-To: <20130107095548.GA6931@redhat.com>
2013/1/7 Stanislaw Gruszka <sgruszka@redhat.com>:
> Since:
>
> commit 2c60db037034d27f8c636403355d52872da92f81
> Author: Eric Dumazet <edumazet@google.com>
> Date: Sun Sep 16 09:17:26 2012 +0000
>
> net: provide a default dev->ethtool_ops
>
> wireless core does not correctly assign ethtool_ops. In order to fix
> the problem, and avoid assigning ethtool_ops on each individual cfg80211
> drivers, we check against default_ethool_ops pointer instead of NULL in
> wireless core.
[...]
You could instead move the assignment of default ethtool_ops to just after
call_netdevice_notifiers(NETDEV_REGISTER) in register_netdevice() or
just after call_netdevice_notifiers(NETDEV_POST_INIT) and initialize the default
wireless ethtool_ops in NETDEV_POST_INIT hook. That will avoid the export.
Either way is good because register_netdevice() is called under RTNL, so
ethtool_ops can't be called until it returns. NETDEV_POST_INIT seams more
natural to me, but it's not a strong opinion.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [RFC PATCH] vsprintf: Add %p*D extension for 80211 SSIDs
From: Joe Perches @ 2013-01-07 17:22 UTC (permalink / raw)
To: Johannes Berg
Cc: John W. Linville, Chen Gang, stas.yakovlev, linux-wireless,
netdev
In-Reply-To: <1357544828.9912.0.camel@jlt4.sipsolutions.net>
On Mon, 2013-01-07 at 08:47 +0100, Johannes Berg wrote:
> On Sun, 2013-01-06 at 20:49 -0800, Joe Perches wrote:
> > On Sun, 2013-01-06 at 19:19 -0800, Joe Perches wrote:
> > > Maybe these days this should be another vsprintf %p extension
> > > like %pM when the DECLARE_MAC_BUF/print_mac uses were converted.
> > >
> > > (or maybe extend %ph for ssids with %*phs, length, array)
> >
> > Maybe like this:
>
> > + * - 'D' For a 80211 SSID, it prints the SSID escaping any non-printable
> > + characters with a leading \ and octal or [0nrt\]
>
> Honestly, I'm not sure it's worth it.
Neither am I really, the only value I see in it is the
reduction in stack consumption by 100 bytes or so per use.
> print_ssid() is used in two or
> three legacy drivers only, not in any modern driver, and is unlikely to
> be used in the more modern drivers due to tracing etc.
Swell. It was just another way to correct those overrun
errors Chen Gang found.
^ permalink raw reply
* Re: [patch net] ethtool: set addr_assign_type to NET_ADDR_SET when addr is passed on create
From: Jiri Pirko @ 2013-01-07 17:52 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem, shemminger, sassmann
In-Reply-To: <1357576565.2658.6.camel@bwh-desktop.uk.solarflarecom.com>
Mon, Jan 07, 2013 at 05:36:05PM CET, bhutchings@solarflare.com wrote:
>On Sun, 2013-01-06 at 23:38 +0100, Jiri Pirko wrote:
>> In case user passed address via netlink during create, NET_ADDR_PERM was set.
>> That is not correct so fix this by setting NET_ADDR_SET.
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>Not sure what this has to do with ethtool...
Should have been "netlink" ...
>
>> ---
>> net/core/rtnetlink.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 9969afb..9a419b0 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1667,9 +1667,11 @@ struct net_device *rtnl_create_link(struct net *net,
>>
>> if (tb[IFLA_MTU])
>> dev->mtu = nla_get_u32(tb[IFLA_MTU]);
>> - if (tb[IFLA_ADDRESS])
>> + if (tb[IFLA_ADDRESS]) {
>> memcpy(dev->dev_addr, nla_data(tb[IFLA_ADDRESS]),
>> nla_len(tb[IFLA_ADDRESS]));
>> + dev->addr_assign_type = NET_ADDR_SET;
>> + }
>> if (tb[IFLA_BROADCAST])
>> memcpy(dev->broadcast, nla_data(tb[IFLA_BROADCAST]),
>> nla_len(tb[IFLA_BROADCAST]));
>
>But if the address is not specified for this new device,
>addr_assign_type remains NET_ADD_PERM, right? That sort of makes sense
>in that the 'permanent address' of a software device is all-zeroes. I
>would prefer to have a way for devices to deny having a permanent
>address, but it's a bit late to change that now.
Well if soft-dev uses eth_hw_addr_random() (like team, bridge,
macvlan, etc) it is set to NET_ADDR_RANDOM
And that is exactly the right way to deny having perm address.
>
>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply
* Re: [patch net-next] ethtool: consolidate work with ethtool_ops
From: Jiri Pirko @ 2013-01-07 18:11 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem, richardcochran, greearb, jeffrey.t.kirsher
In-Reply-To: <1357575542.2658.1.camel@bwh-desktop.uk.solarflarecom.com>
Mon, Jan 07, 2013 at 05:19:02PM CET, bhutchings@solarflare.com wrote:
>On Mon, 2013-01-07 at 13:26 +0100, Jiri Pirko wrote:
>> No need to check if ethtool_ops == NULL since it can't be.
>> Use local variable "ops" in functions where it is present
>> instead of dev->ethtool_ops
>> Introduce local variable "ops" in functions where dev->ethtool_ops is used
>> many times.
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>> net/core/ethtool.c | 45 +++++++++++++++++++++------------------------
>> 1 file changed, 21 insertions(+), 24 deletions(-)
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index a870543..08c213e 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>[...]
>> @@ -1082,9 +1083,10 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
>> {
>> struct ethtool_value id;
>> static bool busy;
>> + const struct ethtool_ops *ops = dev->ethtool_ops;
>> int rc;
>>
>> - if (!dev->ethtool_ops->set_phys_id)
>> + if (ops->set_phys_id)
>> return -EOPNOTSUPP;
>[...]
>
>This condition is inverted.
Thanks, will fix this in v2
>
>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox