* Re: balance-alb and arp-reply stomping
From: Jay Vosburgh @ 2013-01-08 17:14 UTC (permalink / raw)
To: Matthew O'Connor; +Cc: netdev
In-Reply-To: <CAK8PzV=GN0+txo=ixnKGGUXEeJMfd8=Ce=bSReg_oJXScbs79g@mail.gmail.com>
Matthew O'Connor <liquidhorse@gmail.com> wrote:
>Hi! I've encountered what appears to be a known "issue" with
>balance-alb, whereby when a bond configured thus is put into a bridge
>with virtual ethernet adapters, on reply to arp requests the bond
>appears to "stomp" the MAC of the outgoing replies with one of its own
>adapters' MACs. The consequence seems to be intermittent
>connectivity, easily witnessed by ping-loss early in a virtual
>adapter's life. Checking the ARP cache on another machine shows that
>the virtual adapter's MAC has been replaced with one of the bond
>slaves'. Other bonding modes do not exhibit this behavior.
>
>I was wondering if this is something that had been brought up before
>for fixing, and whether or not you would accept a patch if an
>appropriate fix was implemented? My naive understanding would suggest
>checking the outgoing reply against a table of known slaves, and if
>the MAC did not exist there then map it to a slave and transmit the
>reply unmodified.
This should be fixed in current kernels (to have balanace-alb
mode not modify ARPs that do not originate locally); I see the patch in
the 3.8-rc2 source, but not in the linux-3.7.1 source.
You could apply the patch to an older kernel, it's pretty
simple. The patch itself can be found here:
http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=patch;h=567b871e503316b0927e54a3d7c86d50b722d955
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH 1/2] ipv6: avoid blackhole and prohibited entries upon prefix purge [v2]
From: YOSHIFUJI Hideaki @ 2013-01-08 17:18 UTC (permalink / raw)
To: nicolas.dichtel
Cc: Romain KUNTZ, netdev, Eric Dumazet, davem, YOSHIFUJI Hideaki
In-Reply-To: <50EC47AD.8000801@6wind.com>
Nicolas Dichtel wrote:
> Le 08/01/2013 12:38, Romain KUNTZ a écrit :
>> On Jan 7, 2013, at 16:43 , Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>>> 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?
>>
>>
>> I should have given more details from the beginning, my mistake. The scenario where this happens is quite simple:
>>
>> - install a blackhole rule (e.g. "from 2001:db8::1000 blackhole" - the source address does not matter at all) with the FIB_RULE_FIND_SADDR flag set (setting this flag is not possible with iproute2, but for test purpose you can use the enclosed patch against the latest iproute2 tree and then use "./ip -6 rule add from 2001:db8::1000/128 blackhole prio 1000").
>>
>> - try to delete an address from one of your interface (any address, it can be different from the one you used for the blackhole rule): "ip -6 addr del <v6-addr>/64 dev eth<x>"
>>
>> and you get an Oops. When trying to remove the connected prefix, the fib6_rule_match() function will match the blackhole rule because RT6_LOOKUP_F_HAS_SADDR is not set and FIB_RULE_FIND_SADDR is set.
>>
>> With your proposal, the Oops is fixed but the connected prefix route is not deleted. With my initial patch, the Oops is fixed and the connected prefix route is also deleted.
> Ok, I get it. I thin,there is two bugs: the oops and the wrong lookup.
>
> Your proposal fix only a particular case. Try this (with your ip route2 patch):
> ip -6 addr add 2002::1/64 dev eth0
> ip -6 route add 2002::/64 table 257 dev eth0
> ip -6 addr del 2002::1/64 dev eth0
>
> The route deleted is not the connected prefix, but the route added in table 257.
> The connected prefix is still here in the main table. It's not what we want.
> Maybe the lookup should be done directly into the right table, ie table RT6_TABLE_PREFIX. What do you think?
I agree. I think we can use addrconf_get_prefix_route() here.
--yoshfuji
^ permalink raw reply
* Proxy ARP causing Neighbour table overflow
From: Ian Pilcher @ 2013-01-08 17:26 UTC (permalink / raw)
To: netdev
I am using proxy ARP to divide my home network into several different
subnets, while allowing them all to communicate through my residential
"router". I am seeing a (very) large number of "Neighbour table over-
flow" messages, although I haven't noticed any impact on connectivity or
performance.
(If you're not familiar with proxy ARP, it's pretty cool. See
http://en.wikipedia.org/wiki/Proxy_ARP.)
Some details on my setup:
* I have configured my residential router (Verizon FiOS) to use all of
172.31.0.0/16 as my home network.
* The routers internal IP address is 172.31.255.254. As expected its
subnet mask is 255.255.0.0.
* I have configured the router's built-in DHCP server to provide
addresses within the range 172.31.255.151 - 172.31.255.253 (along with
a subnet mask of 255.255.0.0). Normally, the only devices using these
addresses are my FiOS set-top boxes.
* My subnets run on separate VLANs. Routing between them is provided by
a small server running CentOS 6.3 (32-bit). The server has a dual-
port NIC, and it is using mode 6 (balance-alb) bonding. Thus, the
interfaces on the server range from bond0.249 - bond0.255.
* The "upstream" IP address of my server (on bond0.255) is
172.31.255.1; it's subnet mask is 255.255.255.0. (Note the
difference from the router's subnet mask.)
* The server acts as the default gateway for the other subnets --
172.31.249.0/24 (on bond0.249, 172.31.249.254) through 172.31.254.0/24
(on bond0.254, 172.31.249.254).
* I have set "net.ipv4.conf.bond0/255.proxy_arp = 1" in
/etc/sysctl.conf. When the server sees an ARP request on bond0.255
for an address in the range 172.31.249.1 - 172.31.254.254, it responds
with its own MAC (actually, one of its two MACs because of the way
mode 6 bonding works).
Despite all this complexity (for a home network at least), I really
don't have a huge number of devices. Running "arp -n | wc -l" on the
server shows that it has 15 entries in its ARP cache right now, which is
about normal. It may go up a bit when I spin up a bunch of VMs, but not
by that much.
So why am I getting the "Neighbour table overflow" messages. Everything
I can Google up on this messages indicates that it happens with "large,
flat networks" with a lot of users. I do sort of have a large flat
network, but that's really only true from the router's point of view.
(I should note that I only recently configured Ethernet bonding, I was
seeing the "Neighbour table overflow" messages when I was using only a
single interface.)
Any ideas?
Thanks!
--
========================================================================
Ian Pilcher arequipeno@gmail.com
Sometimes there's nothing left to do but crash and burn...or die trying.
========================================================================
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Florian Fainelli @ 2013-01-08 17:36 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Neil Horman, netdev, David Miller, Linus Torvalds, linux-sctp
In-Reply-To: <50EAEE64.5010300@gmail.com>
Le 01/07/13 16:48, Vlad Yasevich a écrit :
> 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.
Right, since none of the config symbols actually exist when migrating
from and oldconfig we are still being prompted, which is just fine
actually. Having the config symbol being the default a member of the
choice/endchoice section is what should be expected.
--
Florian
^ permalink raw reply
* Re: IPV4 TCP connection reset using iperf
From: Rick Jones @ 2013-01-08 17:39 UTC (permalink / raw)
To: Madhvapathi Sriram; +Cc: netdev
In-Reply-To: <CAAvRe=izMJWEFDcZYEoJCY2Em7NQKco4_6x1L7t4h2XSJf6_Sg@mail.gmail.com>
On 01/08/2013 06:41 AM, Madhvapathi Sriram wrote:
> Hi,
>
> I have recently migrated to kernel version 3.6.10 from 3.2.1. I was
> running iperf to routinely measure TCP throughput and I have been
> facing problems eversince. I am using a wireless interface.
>
> wireless client: iperf -s -i 1 -w 1024K
> wireless server: iperf -c 192.168.1.1 -i -1 -w 1024 -t 600
>
> While, the connection is maintained for sometime and the perf logs
> keep going. Randomly, the connection breaks with the server side
> resetting the connection. The tcp dump on the client shows RST sent by
> the server. Switching back to 3.2.1 works. This issue starts from
> kernel version 3.5 onnwards.
>
> I have tried to probe along some points to take a look at the air
> logs, tcpdump on either sides but to no clue - everything seems normal
> like, the tcp window values, very less/negligible retransmissions and
> so on. The RST is set abruptly and randomly (no definite time - may
> happen randomly).
>
> I am looking for some suggestions or pointers towards analyzing the issue.
I cannot imagine that iperf bulk transfer would look all that much
different from netperf bulk transfer, but I suppose you could see if a
netperf TCP_STREAM test with similar configuration settings encounters
the same RST issues. Or, for that matter, an equally long-lived FTP or
scp transfer.
rick jones
^ permalink raw reply
* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
From: Jiri Pirko @ 2013-01-08 17:45 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, edumazet, kuznet, jmorris, yoshfuji, kaber, netdev, pavlix
In-Reply-To: <7abb4995-17a4-49ca-b0a7-41af6ff763e1@tahiti.vyatta.com>
Tue, Jan 08, 2013 at 06:11:55PM CET, stephen.hemminger@vyatta.com wrote:
>
>> When IFF_NOARP is set on a device, dev->dev_addr is used as *dst*
>> addr of sent frames. That does not make sense. Use rather bcast
>> address
>> instead.
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>What did you test this on? I think this may have been
>intentional to avoid broadcasting.
Thanks for looking at this Stephen.
I tested this on two boxes connected via ethernet.
I believe this may have been intentional, but what sense does it have to
use dev_addr as destination address? That is what I do not understand.
Also, what is the issue with sending all packets to broadcast when NOARP is
set? In my opinion, it only makes sense.
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Florian Fainelli @ 2013-01-08 17:48 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Neil Horman, netdev, David Miller, Linus Torvalds, linux-sctp
In-Reply-To: <50EAFA2A.3000200@gmail.com>
Le 01/07/13 17:39, Vlad Yasevich a écrit :
>>
>>> 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.
Right, the issue is as simple as there is no default *valid* config
symbol being proposed. In any case Neil's patch changes both config
symbol names such that they are non-existent when transitionning from a
pre-3.8-rc config and a 3.8-rc config. The following patch fixes this
for me:
From c8e277512bafafc009a3fbf889c78d6b6a06d5fa Mon Sep 17 00:00:00 2001
From: Florian Fainelli <florian@openwrt.org>
Date: Mon, 7 Jan 2013 14:26:15 +0100
Subject: [PATCH] sctp: fix typo in default SCTP cookie choice
Commit 0d0863b0 (sctp: Change defaults on cookie hmac selection)
introduced a choice configuration option to select the default SCTP
cookie hashing algorithm, a typo was introduced for the default choice.
This is an issue when running make oldconfig because an explicit choice
number must be entered since no default is available. This patch fixes
the typo, thus providing a valid default choice.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
net/sctp/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
index c262106..7521d94 100644
--- a/net/sctp/Kconfig
+++ b/net/sctp/Kconfig
@@ -68,7 +68,7 @@ config SCTP_DBG_OBJCNT
If unsure, say N
choice
prompt "Default SCTP cookie HMAC encoding"
- default SCTP_COOKIE_HMAC_MD5
+ default SCTP_DEFAULT_COOKIE_HMAC_MD5
help
This option sets the default sctp cookie hmac algorithm
when in doubt select 'md5'
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-08 18:08 UTC (permalink / raw)
To: Florian Fainelli
Cc: Neil Horman, netdev, David Miller, Linus Torvalds, linux-sctp,
Alex Elder
In-Reply-To: <50EC5BE7.3090401@openwrt.org>
On 01/08/2013 12:48 PM, Florian Fainelli wrote:
> Le 01/07/13 17:39, Vlad Yasevich a écrit :
>>>
>>>> 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.
>
> Right, the issue is as simple as there is no default *valid* config
> symbol being proposed. In any case Neil's patch changes both config
> symbol names such that they are non-existent when transitionning from a
> pre-3.8-rc config and a 3.8-rc config. The following patch fixes this
> for me:
>
> From c8e277512bafafc009a3fbf889c78d6b6a06d5fa Mon Sep 17 00:00:00 2001
> From: Florian Fainelli <florian@openwrt.org>
> Date: Mon, 7 Jan 2013 14:26:15 +0100
> Subject: [PATCH] sctp: fix typo in default SCTP cookie choice
>
> Commit 0d0863b0 (sctp: Change defaults on cookie hmac selection)
> introduced a choice configuration option to select the default SCTP
> cookie hashing algorithm, a typo was introduced for the default choice.
> This is an issue when running make oldconfig because an explicit choice
> number must be entered since no default is available. This patch fixes
> the typo, thus providing a valid default choice.
>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
Hi Florian
Alex Alder already sent the patch that Neil and I both acked, but I see
that Alex never sent it to netdev (just checked the headers).
Oh well... this is the same change that Alex sent, so ACK, and now it
can get into the tree.
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Alex, please be sure to CC your patches to netdev as well.
Thanks
-vlad
> ---
> net/sctp/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
> index c262106..7521d94 100644
> --- a/net/sctp/Kconfig
> +++ b/net/sctp/Kconfig
> @@ -68,7 +68,7 @@ config SCTP_DBG_OBJCNT
> If unsure, say N
> choice
> prompt "Default SCTP cookie HMAC encoding"
> - default SCTP_COOKIE_HMAC_MD5
> + default SCTP_DEFAULT_COOKIE_HMAC_MD5
> help
> This option sets the default sctp cookie hmac algorithm
> when in doubt select 'md5'
^ permalink raw reply
* Re: [PATCH] sunrpc: verbs: Avoid 1kb stack
From: Joe Perches @ 2013-01-08 18:09 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Trond Myklebust, David S. Miller,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tom Tucker,
steved-H+wXaHxf7aLQT0dZR+AlfA, Tom Talpey
In-Reply-To: <20130108144604.GB16343-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On Tue, 2013-01-08 at 09:46 -0500, J. Bruce Fields wrote:
> On Mon, Jan 07, 2013 at 11:41:36AM -0800, Joe Perches wrote:
> > 16 * 64 is a bit much.
> > Use kmalloc_array instead.
>
> I thought there was some reason we didn't do this.
>
> Grepping up through the callers.... It looks like the result is
> xprt_rdma_send_request returns -EIO, and as far as I can tell that gets
> passed up to the application on the client. That doesn't sound right.
No worries, it was just a warning I noticed when I
did an allmodconfig compilation.
Perhaps a comment there might be appropriate instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/5] kfifo: log based kfifo API
From: Dmitry Torokhov @ 2013-01-08 18:16 UTC (permalink / raw)
To: Yuanhan Liu
Cc: linux-kernel, Stefani Seibold, Andrew Morton, linux-omap,
linuxppc-dev, platform-driver-x86, linux-input, linux-iio,
linux-rdma, linux-media, linux-mmc, linux-mtd, libertas-dev,
linux-wireless, netdev, linux-pci, open-iscsi, linux-scsi, devel,
linux-serial, linux-usb, linux-mm, dccp, linux-sctp
In-Reply-To: <1357657073-27352-6-git-send-email-yuanhan.liu@linux.intel.com>
Hi Yuanhan,
On Tue, Jan 08, 2013 at 10:57:53PM +0800, Yuanhan Liu wrote:
> The current kfifo API take the kfifo size as input, while it rounds
> _down_ the size to power of 2 at __kfifo_alloc. This may introduce
> potential issue.
>
> Take the code at drivers/hid/hid-logitech-dj.c as example:
>
> if (kfifo_alloc(&djrcv_dev->notif_fifo,
> DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct dj_report),
> GFP_KERNEL)) {
>
> Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct dj_report)
> is 15.
>
> Which means it wants to allocate a kfifo buffer which can store 8
> dj_report entries at once. The expected kfifo buffer size would be
> 8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the
> size to rounddown_power_of_2(120) = 64, and then allocate a buf
> with 64 bytes, which I don't think this is the original author want.
>
> With the new log API, we can do like following:
>
> int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS *
> sizeof(struct dj_report));
>
> if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order, GFP_KERNEL)) {
>
> This make sure we will allocate enough kfifo buffer for holding
> DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries.
Why don't you simply change __kfifo_alloc to round the allocation up
instead of down?
Thanks.
--
Dmitry
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Alex Elder @ 2013-01-08 18:20 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Florian Fainelli, Neil Horman, netdev, David Miller,
Linus Torvalds, linux-sctp
In-Reply-To: <50EC60B8.4070205@gmail.com>
On 01/08/2013 12:08 PM, Vlad Yasevich wrote:
> On 01/08/2013 12:48 PM, Florian Fainelli wrote:
>> Le 01/07/13 17:39, Vlad Yasevich a écrit :
>>>>
>>>>> 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.
>>
>> Right, the issue is as simple as there is no default *valid* config
>> symbol being proposed. In any case Neil's patch changes both config
>> symbol names such that they are non-existent when transitionning from a
>> pre-3.8-rc config and a 3.8-rc config. The following patch fixes this
>> for me:
>>
>> From c8e277512bafafc009a3fbf889c78d6b6a06d5fa Mon Sep 17 00:00:00 2001
>> From: Florian Fainelli <florian@openwrt.org>
>> Date: Mon, 7 Jan 2013 14:26:15 +0100
>> Subject: [PATCH] sctp: fix typo in default SCTP cookie choice
>>
>> Commit 0d0863b0 (sctp: Change defaults on cookie hmac selection)
>> introduced a choice configuration option to select the default SCTP
>> cookie hashing algorithm, a typo was introduced for the default choice.
>> This is an issue when running make oldconfig because an explicit choice
>> number must be entered since no default is available. This patch fixes
>> the typo, thus providing a valid default choice.
>>
>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
>
> Hi Florian
>
> Alex Alder already sent the patch that Neil and I both acked, but I see
> that Alex never sent it to netdev (just checked the headers).
>
> Oh well... this is the same change that Alex sent, so ACK, and now it
> can get into the tree.
>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
>
> Alex, please be sure to CC your patches to netdev as well.
Sorry, I just sent my messages to the addresses listed
on the commit that introduced the problem this fixed.
Linus already pulled in the patch.
-Alex
> Thanks
> -vlad
>
>> ---
>> net/sctp/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>> index c262106..7521d94 100644
>> --- a/net/sctp/Kconfig
>> +++ b/net/sctp/Kconfig
>> @@ -68,7 +68,7 @@ config SCTP_DBG_OBJCNT
>> If unsure, say N
>> choice
>> prompt "Default SCTP cookie HMAC encoding"
>> - default SCTP_COOKIE_HMAC_MD5
>> + default SCTP_DEFAULT_COOKIE_HMAC_MD5
>> help
>> This option sets the default sctp cookie hmac algorithm
>> when in doubt select 'md5'
>
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-08 18:28 UTC (permalink / raw)
To: Alex Elder
Cc: Florian Fainelli, Neil Horman, netdev, David Miller,
Linus Torvalds, linux-sctp
In-Reply-To: <50EC6378.7020505@inktank.com>
On 01/08/2013 01:20 PM, Alex Elder wrote:
> On 01/08/2013 12:08 PM, Vlad Yasevich wrote:
>> On 01/08/2013 12:48 PM, Florian Fainelli wrote:
>>> Le 01/07/13 17:39, Vlad Yasevich a écrit :
>>>>>
>>>>>> 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.
>>>
>>> Right, the issue is as simple as there is no default *valid* config
>>> symbol being proposed. In any case Neil's patch changes both config
>>> symbol names such that they are non-existent when transitionning from a
>>> pre-3.8-rc config and a 3.8-rc config. The following patch fixes this
>>> for me:
>>>
>>> From c8e277512bafafc009a3fbf889c78d6b6a06d5fa Mon Sep 17 00:00:00 2001
>>> From: Florian Fainelli <florian@openwrt.org>
>>> Date: Mon, 7 Jan 2013 14:26:15 +0100
>>> Subject: [PATCH] sctp: fix typo in default SCTP cookie choice
>>>
>>> Commit 0d0863b0 (sctp: Change defaults on cookie hmac selection)
>>> introduced a choice configuration option to select the default SCTP
>>> cookie hashing algorithm, a typo was introduced for the default choice.
>>> This is an issue when running make oldconfig because an explicit choice
>>> number must be entered since no default is available. This patch fixes
>>> the typo, thus providing a valid default choice.
>>>
>>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
>>
>> Hi Florian
>>
>> Alex Alder already sent the patch that Neil and I both acked, but I see
>> that Alex never sent it to netdev (just checked the headers).
>>
>> Oh well... this is the same change that Alex sent, so ACK, and now it
>> can get into the tree.
>>
>> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
>>
>> Alex, please be sure to CC your patches to netdev as well.
>
> Sorry, I just sent my messages to the addresses listed
> on the commit that introduced the problem this fixed.
>
> Linus already pulled in the patch.
>
I see.. Then this is a noop ;)
-vlad
> -Alex
>
>> Thanks
>> -vlad
>>
>>> ---
>>> net/sctp/Kconfig | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>> index c262106..7521d94 100644
>>> --- a/net/sctp/Kconfig
>>> +++ b/net/sctp/Kconfig
>>> @@ -68,7 +68,7 @@ config SCTP_DBG_OBJCNT
>>> If unsure, say N
>>> choice
>>> prompt "Default SCTP cookie HMAC encoding"
>>> - default SCTP_COOKIE_HMAC_MD5
>>> + default SCTP_DEFAULT_COOKIE_HMAC_MD5
>>> help
>>> This option sets the default sctp cookie hmac algorithm
>>> when in doubt select 'md5'
>>
>
^ permalink raw reply
* [PATCH net 0/2] net: dev_queue_xmit_nit fixes
From: Daniel Borkmann @ 2013-01-08 18:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Daniel Borkmann
First patch is a fix for the netdev discussion ``PROBLEM: Software injected
vlan tagged packets are unable to be identified using recent BPF
modifications'' and the second one I spotted while doing the first fix.
Daniel Borkmann (2):
net: dev_queue_xmit_nit: fix skb->vlan_tci field value
net: dev_queue_xmit_nit: fix potential NULL ptr dereference
net/core/dev.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
--
1.7.11.7
^ permalink raw reply
* [PATCH net 1/2] net: dev_queue_xmit_nit: fix skb->vlan_tci field value
From: Daniel Borkmann @ 2013-01-08 18:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Daniel Borkmann, Eric Dumazet, Ani Sinha, Jiri Pirko
In-Reply-To: <1357671093-9605-1-git-send-email-dborkman@redhat.com>
VLAN packets that are locally injected through taps will loose their
skb->vlan_tci value when they pass dev_hard_start_xmit and get looped
back to a packet sniffer via dev_queue_xmit_nit. Besides others, this
meta data is used in Linux socket filtering for VLANs. Tested with a
VLAN ancillary ops filter.
Patch is based on a previous version by Jiri Pirko.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ani Sinha <ani@aristanetworks.com>
Cc: Jiri Pirko <jpirko@redhat.com>
Reported-by: Paul Pearce <pearce@cs.berkeley.edu>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/core/dev.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 515473e..723dcd0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1775,6 +1775,19 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
struct packet_type *ptype;
struct sk_buff *skb2 = NULL;
struct packet_type *pt_prev = NULL;
+ struct ethhdr *ehdr;
+
+ /* Network taps could make use of skb->vlan_tci, which got wiped
+ * out. Hence, we need to reset it correctly.
+ */
+ skb_reset_mac_header(skb);
+ ehdr = eth_hdr(skb);
+
+ if (ehdr->h_proto == __constant_htons(ETH_P_8021Q)) {
+ skb2 = vlan_untag(skb);
+ if (likely(skb2))
+ skb = skb2;
+ }
rcu_read_lock();
list_for_each_entry_rcu(ptype, &ptype_all, list) {
--
1.7.11.7
^ permalink raw reply related
* [PATCH net 2/2] net: dev_queue_xmit_nit: fix potential NULL ptr dereference
From: Daniel Borkmann @ 2013-01-08 18:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Daniel Borkmann, Changli Gao, Eric Dumazet
In-Reply-To: <1357671093-9605-1-git-send-email-dborkman@redhat.com>
Commit 71d9dec24dce548bf699815c976cf063ad9257e2 (``net: increase
skb->users instead of skb_clone()'') introduced a skb_clone in
dev_queue_xmit_nit that, when NULL, leaves the loop, but can still
be injected into pt_prev->func().
Cc: Changli Gao <xiaosuo@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 723dcd0..6c35c33 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1827,7 +1827,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
pt_prev = ptype;
}
}
- if (pt_prev)
+ if (skb2 && pt_prev)
pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
rcu_read_unlock();
}
--
1.7.11.7
^ permalink raw reply related
* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
From: Stephen Hemminger @ 2013-01-08 19:10 UTC (permalink / raw)
To: Jiri Pirko
Cc: davem, edumazet, kuznet, jmorris, yoshfuji, kaber, netdev, pavlix
In-Reply-To: <20130108174538.GC1621@minipsycho.orion>
> Tue, Jan 08, 2013 at 06:11:55PM CET, stephen.hemminger@vyatta.com
> wrote:
> >
> >> When IFF_NOARP is set on a device, dev->dev_addr is used as *dst*
> >> addr of sent frames. That does not make sense. Use rather bcast
> >> address
> >> instead.
> >>
> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >
> >What did you test this on? I think this may have been
> >intentional to avoid broadcasting.
>
> Thanks for looking at this Stephen.
>
> I tested this on two boxes connected via ethernet.
>
> I believe this may have been intentional, but what sense does it have
> to
> use dev_addr as destination address? That is what I do not
> understand.
>
> Also, what is the issue with sending all packets to broadcast when
> NOARP is
> set? In my opinion, it only makes sense.
>
It looks like NOARP devices are point-to-point and don't really
have any concept of broadcast.
^ permalink raw reply
* Re: [PATCH net 2/2] net: dev_queue_xmit_nit: fix potential NULL ptr dereference
From: Eric Dumazet @ 2013-01-08 19:22 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, netdev, Changli Gao
In-Reply-To: <1357671093-9605-3-git-send-email-dborkman@redhat.com>
On Tue, 2013-01-08 at 19:51 +0100, Daniel Borkmann wrote:
> Commit 71d9dec24dce548bf699815c976cf063ad9257e2 (``net: increase
> skb->users instead of skb_clone()'') introduced a skb_clone in
> dev_queue_xmit_nit that, when NULL, leaves the loop, but can still
> be injected into pt_prev->func().
>
> Cc: Changli Gao <xiaosuo@gmail.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> net/core/dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 723dcd0..6c35c33 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1827,7 +1827,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
> pt_prev = ptype;
> }
> }
> - if (pt_prev)
> + if (skb2 && pt_prev)
> pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
> rcu_read_unlock();
> }
My opinion is this patch is not needed.
pt_prev can be set only if skb2 is not NULL
^ permalink raw reply
* Re: [PATCH net 2/2] net: dev_queue_xmit_nit: fix potential NULL ptr dereference
From: Daniel Borkmann @ 2013-01-08 19:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Changli Gao
In-Reply-To: <1357672943.18156.328.camel@edumazet-glaptop>
On 01/08/2013 08:22 PM, Eric Dumazet wrote:
> On Tue, 2013-01-08 at 19:51 +0100, Daniel Borkmann wrote:
>> Commit 71d9dec24dce548bf699815c976cf063ad9257e2 (``net: increase
>> skb->users instead of skb_clone()'') introduced a skb_clone in
>> dev_queue_xmit_nit that, when NULL, leaves the loop, but can still
>> be injected into pt_prev->func().
>>
>> Cc: Changli Gao <xiaosuo@gmail.com>
>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> ---
>> net/core/dev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 723dcd0..6c35c33 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1827,7 +1827,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>> pt_prev = ptype;
>> }
>> }
>> - if (pt_prev)
>> + if (skb2 && pt_prev)
>> pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
>> rcu_read_unlock();
>> }
>
> My opinion is this patch is not needed.
>
> pt_prev can be set only if skb2 is not NULL
Agreed, I missed that, thanks.
^ permalink raw reply
* Re: [PATCH net 1/2] net: dev_queue_xmit_nit: fix skb->vlan_tci field value
From: Ani Sinha @ 2013-01-08 19:54 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, netdev, Eric Dumazet, Jiri Pirko
In-Reply-To: <1357671093-9605-2-git-send-email-dborkman@redhat.com>
Agreed with the fix. This fixes the issue introduced by this change :l
On Tue, Jan 8, 2013 at 10:51 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> VLAN packets that are locally injected through taps will loose their
> skb->vlan_tci value when they pass dev_hard_start_xmit and get looped
> back to a packet sniffer via dev_queue_xmit_nit. Besides others, this
> meta data is used in Linux socket filtering for VLANs. Tested with a
> VLAN ancillary ops filter.
>
> Patch is based on a previous version by Jiri Pirko.
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Ani Sinha <ani@aristanetworks.com>
> Cc: Jiri Pirko <jpirko@redhat.com>
> Reported-by: Paul Pearce <pearce@cs.berkeley.edu>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Ani Sinha <ani@aristanetworks.com>
^ permalink raw reply
* Re: [PATCH net 1/2] net: dev_queue_xmit_nit: fix skb->vlan_tci field value
From: Eric Dumazet @ 2013-01-08 20:04 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, netdev, Ani Sinha, Jiri Pirko
In-Reply-To: <1357671093-9605-2-git-send-email-dborkman@redhat.com>
On Tue, 2013-01-08 at 19:51 +0100, Daniel Borkmann wrote:
> VLAN packets that are locally injected through taps will loose their
> skb->vlan_tci value when they pass dev_hard_start_xmit and get looped
> back to a packet sniffer via dev_queue_xmit_nit. Besides others, this
> meta data is used in Linux socket filtering for VLANs. Tested with a
> VLAN ancillary ops filter.
>
> Patch is based on a previous version by Jiri Pirko.
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Ani Sinha <ani@aristanetworks.com>
> Cc: Jiri Pirko <jpirko@redhat.com>
> Reported-by: Paul Pearce <pearce@cs.berkeley.edu>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> net/core/dev.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 515473e..723dcd0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1775,6 +1775,19 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
> struct packet_type *ptype;
> struct sk_buff *skb2 = NULL;
> struct packet_type *pt_prev = NULL;
> + struct ethhdr *ehdr;
> +
> + /* Network taps could make use of skb->vlan_tci, which got wiped
> + * out. Hence, we need to reset it correctly.
> + */
> + skb_reset_mac_header(skb);
> + ehdr = eth_hdr(skb);
> +
> + if (ehdr->h_proto == __constant_htons(ETH_P_8021Q)) {
> + skb2 = vlan_untag(skb);
> + if (likely(skb2))
> + skb = skb2;
> + }
>
> rcu_read_lock();
> list_for_each_entry_rcu(ptype, &ptype_all, list) {
This patch is wrong (it adds a leak), and not needed.
If a packet has no vlan_tci, its for a good reason.
We want sniffer see the packet content as is.
^ permalink raw reply
* Re: ppoll() stuck on POLLIN while TCP peer is sending
From: Eric Wong @ 2013-01-08 20:14 UTC (permalink / raw)
To: Mel Gorman
Cc: linux-mm, netdev, linux-kernel, Rik van Riel, Minchan Kim,
Eric Dumazet, Andrew Morton, Linus Torvalds
In-Reply-To: <20130107223850.GA21311@dcvr.yhbt.net>
Eric Wong <normalperson@yhbt.net> wrote:
> Mel Gorman <mgorman@suse.de> wrote:
> > Right now it's difficult to see how the capture could be the source of
> > this bug but I'm not ruling it out either so try the following (untested
> > but should be ok) patch. It's not a proper revert, it just disables the
> > capture page logic to see if it's at fault.
>
> Things look good so far with your change.
> It's been running 2 hours on a VM and 1 hour on my regular machine.
> Will update again in a few hours (or sooner if it's stuck again).
Things still seem good on my regular machine.
^ permalink raw reply
* Re: [PATCH net 1/2] net: dev_queue_xmit_nit: fix skb->vlan_tci field value
From: Jiri Pirko @ 2013-01-08 20:14 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, netdev, Eric Dumazet, Ani Sinha, Jiri Pirko
In-Reply-To: <1357671093-9605-2-git-send-email-dborkman@redhat.com>
Tue, Jan 08, 2013 at 07:51:32PM CET, dborkman@redhat.com wrote:
>VLAN packets that are locally injected through taps will loose their
>skb->vlan_tci value when they pass dev_hard_start_xmit and get looped
>back to a packet sniffer via dev_queue_xmit_nit. Besides others, this
>meta data is used in Linux socket filtering for VLANs. Tested with a
>VLAN ancillary ops filter.
>
>Patch is based on a previous version by Jiri Pirko.
>
>Cc: Eric Dumazet <eric.dumazet@gmail.com>
>Cc: Ani Sinha <ani@aristanetworks.com>
>Cc: Jiri Pirko <jpirko@redhat.com>
>Reported-by: Paul Pearce <pearce@cs.berkeley.edu>
>Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>---
> net/core/dev.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 515473e..723dcd0 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -1775,6 +1775,19 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
> struct packet_type *ptype;
> struct sk_buff *skb2 = NULL;
> struct packet_type *pt_prev = NULL;
>+ struct ethhdr *ehdr;
>+
>+ /* Network taps could make use of skb->vlan_tci, which got wiped
>+ * out. Hence, we need to reset it correctly.
>+ */
>+ skb_reset_mac_header(skb);
>+ ehdr = eth_hdr(skb);
>+
>+ if (ehdr->h_proto == __constant_htons(ETH_P_8021Q)) {
>+ skb2 = vlan_untag(skb);
>+ if (likely(skb2))
>+ skb = skb2;
>+ }
Hmm, nitpick, I think that better would be to do:
skb = vlan_untag(skb);
if (unlikely(!skb))
return;
I believe that better is to deliver skbs in consistent way and
to do not deliver at all in case of -ENOMEM
^ permalink raw reply
* Re: [PATCH net 1/2] net: dev_queue_xmit_nit: fix skb->vlan_tci field value
From: Jiri Pirko @ 2013-01-08 20:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Daniel Borkmann, David Miller, netdev, Ani Sinha, Jiri Pirko
In-Reply-To: <1357675492.18156.420.camel@edumazet-glaptop>
Tue, Jan 08, 2013 at 09:04:52PM CET, eric.dumazet@gmail.com wrote:
>On Tue, 2013-01-08 at 19:51 +0100, Daniel Borkmann wrote:
>> VLAN packets that are locally injected through taps will loose their
>> skb->vlan_tci value when they pass dev_hard_start_xmit and get looped
>> back to a packet sniffer via dev_queue_xmit_nit. Besides others, this
>> meta data is used in Linux socket filtering for VLANs. Tested with a
>> VLAN ancillary ops filter.
>>
>> Patch is based on a previous version by Jiri Pirko.
>>
>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>> Cc: Ani Sinha <ani@aristanetworks.com>
>> Cc: Jiri Pirko <jpirko@redhat.com>
>> Reported-by: Paul Pearce <pearce@cs.berkeley.edu>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> ---
>> net/core/dev.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 515473e..723dcd0 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1775,6 +1775,19 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>> struct packet_type *ptype;
>> struct sk_buff *skb2 = NULL;
>> struct packet_type *pt_prev = NULL;
>> + struct ethhdr *ehdr;
>> +
>> + /* Network taps could make use of skb->vlan_tci, which got wiped
>> + * out. Hence, we need to reset it correctly.
>> + */
>> + skb_reset_mac_header(skb);
>> + ehdr = eth_hdr(skb);
>> +
>> + if (ehdr->h_proto == __constant_htons(ETH_P_8021Q)) {
>> + skb2 = vlan_untag(skb);
>> + if (likely(skb2))
>> + skb = skb2;
>> + }
>>
>> rcu_read_lock();
>> list_for_each_entry_rcu(ptype, &ptype_all, list) {
>
>This patch is wrong (it adds a leak), and not needed.
>
>If a packet has no vlan_tci, its for a good reason.
>
>We want sniffer see the packet content as is.
The issue is that for exmaple in af_packet the function packet_rcv()
expects vlan_tci to be filled out as that is on RX path ensured by
__netif_receive_skb(). However on TX path, dev_queue_xmit_nit() is
called with vlan_tci cleaned in case the device does not have TX vlan
accel enabled. This patch is trying to fix this difference.
>
>
>
>--
>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 net 1/2] net: dev_queue_xmit_nit: fix skb->vlan_tci field value
From: Eric Dumazet @ 2013-01-08 20:42 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Daniel Borkmann, David Miller, netdev, Ani Sinha, Jiri Pirko
In-Reply-To: <20130108202208.GE1621@minipsycho.orion>
On Tue, 2013-01-08 at 21:22 +0100, Jiri Pirko wrote:
>
> The issue is that for exmaple in af_packet the function packet_rcv()
> expects vlan_tci to be filled out as that is on RX path ensured by
> __netif_receive_skb(). However on TX path, dev_queue_xmit_nit() is
> called with vlan_tci cleaned in case the device does not have TX vlan
> accel enabled. This patch is trying to fix this difference.
I perfectly understood that, and I repeat :
I want to see the difference.
A filter cannot expect skb->vlan_tci is set for all packets.
If a device doesn't have TX vlan accel, vlan_tci is 0.
packet sniffing is already slow, we don't want to force an extra copy
with vlan_untag() killer.
If you want to fix/extend af_packet, do this in af_packet.
^ permalink raw reply
* Re: [PATCH 5/5] kfifo: log based kfifo API
From: Andy Walls @ 2013-01-08 21:10 UTC (permalink / raw)
To: Dmitry Torokhov, Yuanhan Liu
Cc: linux-kernel, Stefani Seibold, Andrew Morton, linux-omap,
linuxppc-dev, platform-driver-x86, linux-input, linux-iio,
linux-rdma, linux-media, linux-mmc, linux-mtd, libertas-dev,
linux-wireless, netdev, linux-pci, open-iscsi, linux-scsi, devel,
linux-serial, linux-usb, linux-mm, dccp, linux-sctp
In-Reply-To: <20130108181645.GA7972@core.coreip.homeip.net>
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>Hi Yuanhan,
>
>On Tue, Jan 08, 2013 at 10:57:53PM +0800, Yuanhan Liu wrote:
>> The current kfifo API take the kfifo size as input, while it rounds
>> _down_ the size to power of 2 at __kfifo_alloc. This may introduce
>> potential issue.
>>
>> Take the code at drivers/hid/hid-logitech-dj.c as example:
>>
>> if (kfifo_alloc(&djrcv_dev->notif_fifo,
>> DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct
>dj_report),
>> GFP_KERNEL)) {
>>
>> Where, DJ_MAX_NUMBER_NOTIFICATIONS is 8, and sizeo of(struct
>dj_report)
>> is 15.
>>
>> Which means it wants to allocate a kfifo buffer which can store 8
>> dj_report entries at once. The expected kfifo buffer size would be
>> 8 * 15 = 120 then. While, in the end, __kfifo_alloc will turn the
>> size to rounddown_power_of_2(120) = 64, and then allocate a buf
>> with 64 bytes, which I don't think this is the original author want.
>>
>> With the new log API, we can do like following:
>>
>> int kfifo_size_order = order_base_2(DJ_MAX_NUMBER_NOTIFICATIONS *
>> sizeof(struct dj_report));
>>
>> if (kfifo_alloc(&djrcv_dev->notif_fifo, kfifo_size_order,
>GFP_KERNEL)) {
>>
>> This make sure we will allocate enough kfifo buffer for holding
>> DJ_MAX_NUMBER_NOTIFICATIONS dj_report entries.
>
>Why don't you simply change __kfifo_alloc to round the allocation up
>instead of down?
>
>Thanks.
>
>--
>Dmitry
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media"
>in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dmitry,
I agree. I don't see the benefit in pushing up the change to a kfifo internal decision/problem to many different places in the kernel.
Regards,
Andy
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ 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