* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-26 9:49 UTC (permalink / raw)
To: Daniel Borkmann, Stathis Voukelatos
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <54C22E68.1080601@redhat.com>
On 23/01/15 11:20, Daniel Borkmann wrote:
> On 01/23/2015 11:07 AM, Stathis Voukelatos wrote:
>> This patch adds support the Ethernet Packet Sniffer H/W module
>> developed by Linn Products Ltd and found in the IMG Pistachio SoC.
>> The module allows Ethernet packets to be parsed, matched against
>> a user-defined pattern and timestamped. It sits between a 100M
>> Ethernet MAC and PHY and is completely passive with respect to
>> Ethernet frames.
>>
>> Matched packet bytes and timestamp values are returned through a
>> FIFO. Timestamps are provided to the module through an externally
>> generated Gray-encoded counter.
>>
>> The command pattern for packet matching is stored in module RAM
>> and consists of a sequence of 16-bit entries. Each entry includes
>> an 8-bit command code and and 8-bit data value. Valid command
>> codes are:
>> 0 - Don't care
>> 1 - Match: packet data must match command string byte
>> 2 - Copy: packet data will be copied to FIFO
>> 3 - Match/Stamp: if packet data matches string byte, a timestamp
>> is copied into the FIFO
>> 4 - Copy/Done: packet data will be copied into the FIFO.
>> This command terminates the command string.
>>
>> The driver consists of two modules:
>> - Core: it provides an API to user space using the Generic Netlink
>> framework. Specific backend implementations, like the
>> Ethernet Packet Sniffer, register one or more channels
>> with the Core. For each channel a Genl family is created.
>> User space can access a channel by sending Genl messages
>> to the Genl family associated with the channel. Packet
>> matching events are multicast.
>>
>> - Ethernet Packet Sniffer backend: provides the driver for the
>> Linn Ethernet Packet Sniffer H/W modules.
>>
>> The split between a core and backend modules allows software-only
>> implementations to be added for platforms where no H/W support
>> is available.
>>
>> Based on 3.19-rc5
>>
>> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> Please have a look at packet sockets, they offer already all the
> functionality (if not more) your driver interface to the user space
> resembles, are transparent to the underlying hardware, and easily
> can cope with 100Mbit.
>
> If I understand this correctly, you are effectively introducing a
> parallel API *next* to packet sockets to user space that we have to
> maintain forever ...
>
> Thanks !
>
Hello Daniel. Thank you for your feedback.
Packet sockets could also be used for the driver interface to
user space, however I think that both approaches would require the same
amount of maintenance. We need to maintain a protocol consisting of
a set of messages or commands that user space can use to communicate
with the driver in order to configure the H/W and retrieve results.
We could use packet sockets to send those messages too, but I thought
netlink already provides a message exchange framework that we could
make use of.
^ permalink raw reply
* [PATCH net-next] vlan: take link netns into account in vlan_newlink()
From: Nicolas Dichtel @ 2015-01-26 9:44 UTC (permalink / raw)
To: netdev; +Cc: davem, Nicolas Dichtel
When IFLA_LINK_NETNSID is set, the ifindex from IFLA_LINK comes from another
netns.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
net/8021q/vlan_netlink.c | 13 ++++++++++++-
net/core/net_namespace.c | 1 +
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index c92b52f37d38..e7875c7f3cb4 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -118,6 +118,7 @@ static int vlan_newlink(struct net *src_net, struct net_device *dev,
{
struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
struct net_device *real_dev;
+ struct net *link_net;
__be16 proto;
int err;
@@ -126,7 +127,17 @@ static int vlan_newlink(struct net *src_net, struct net_device *dev,
if (!tb[IFLA_LINK])
return -EINVAL;
- real_dev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
+ if (tb[IFLA_LINK_NETNSID]) {
+ int id = nla_get_s32(tb[IFLA_LINK_NETNSID]);
+
+ link_net = get_net_ns_by_id(src_net, id);
+ if (!link_net)
+ return -EINVAL;
+ } else {
+ link_net = get_net(src_net);
+ }
+ real_dev = __dev_get_by_index(link_net, nla_get_u32(tb[IFLA_LINK]));
+ put_net(link_net);
if (!real_dev)
return -ENODEV;
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index b7bde551ef76..8e24f974c665 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -219,6 +219,7 @@ struct net *get_net_ns_by_id(struct net *net, int id)
return peer;
}
+EXPORT_SYMBOL(get_net_ns_by_id);
/*
* setup_net runs the initializers for the network namespace object.
--
2.2.2
^ permalink raw reply related
* Re: [PATCH net-next 0/6] netns: advertise netns via netlink
From: Nicolas Dichtel @ 2015-01-26 9:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20150123.175142.618148236781483266.davem@davemloft.net>
Le 24/01/2015 02:51, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Tue, 20 Jan 2015 15:15:41 +0100
>
>>
>> The first patch of the serie fix a bug of the previous serie (present in
>> net-next only).
>> The rest of the serie adds an attribute to advertise the peer netns for
>> rtnetlink messages where this information is needed by userland to be able to
>> interpret fully the received message.
>
> "series" has a final 's'. Well, as least you consistently misspell it
> 3 times :-)
Thanks for noticing it.
For the story, it comes from the french word 'serie' which don't end with a 's'
;-)
^ permalink raw reply
* Re: [PATCH net-next 1/7] r8152: adjust rx_bottom
From: Scott Feldman @ 2015-01-26 9:14 UTC (permalink / raw)
To: Hayes Wang
Cc: David Miller, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
nic_swsd, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2EE6E76-JIZ+AM9kKNzuvTFwvkocLypo8c9IxeqyAjHCUHv49ws@public.gmane.org>
On Mon, Jan 19, 2015 at 6:48 PM, Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org> wrote:
> David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
>> Sent: Tuesday, January 20, 2015 5:14 AM
> [...]
>> >> - r8152_submit_rx(tp, agg, GFP_ATOMIC);
>> >> + if (!ret) {
>> >> + ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
>> >> + } else {
>> >> + urb->actual_length = 0;
>> >> + list_add_tail(&agg->list, next);
>> >
>> > Do you need a spin_lock_irqsave(&tp->rx_lock, flags) around this?
>>
>> Indeed, and rtl_start_rx() seems to also access agg->list without
>> proper locking.
>
> It is unnecessary because I deal with them in a local list_head. My steps are
> 1. Move the whole list from tp->rx_done to local rx_queue. (with spin lock)
> 2. dequeue/enqueue the lists in rx_queue.
> 3. Move the lists in rx_queue to tp->rx_done if it is necessary. (spin lock)
> For step 2, it wouldn't have race, because the list_head is local and no other
> function would change it. Therefore, I don't think it needs the spin lock.
Sorry guys, I think I made a mistake in my review and caused some
confusion/grief.
My mistake was getting the params to list_add_tail() backwards. It's
list_add_tail(entry, head). I saw list_add_tail(&agg->list, next) and
was fooled into thinking agg->list was the list getting appended with
the entry 'next'. It's the opposite. Duh. So locking isn't needed
because the list is indeed local.
-scott
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: IPsec workshop at netdev01?
From: Steffen Klassert @ 2015-01-26 9:11 UTC (permalink / raw)
To: Fan Du; +Cc: netdev, Jamal Hadi Salim, Herbert Xu, David Miller, Du, Fan
In-Reply-To: <54AF677E.9080108@gmail.com>
On Fri, Jan 09, 2015 at 01:30:38PM +0800, Fan Du wrote:
> 于 2015年01月06日 18:19, Steffen Klassert 写道:
> >
> >- We still lack a 32/64 bit compatibiltiy layer for IPsec, this issue
> > comes up from time to time. Some solutions were proposed in the past
> > but all had problems. The current behaviour is broken if someone tries
> > to configure IPsec with 32 bit tools on a 64 bit machine. Can we get
> > this right somehow or is it better to just return an error in this case?
>
> Before a clean solution show up, I think it's better to warn user in some way
> like http://patchwork.ozlabs.org/patch/323842/ did. Otherwise, many people
> who stuck there will always spend time and try to fix this issue in whatever way.
Yes, this is the first thing we should do. I'm willing to accept a patch :)
>
> >- Changing the system time can lead to unexpected SA lifetime changes. The
> > discussion on the list did not lead to a conclusion on how to fix this.
> > What is the best way to get this fixed?
>
> I rise this issue long ago before, the culprit is SA lifetime is marked by wall clock.
> In a reasonable way it should be marked as monotonic boot time(counting suspend time
> as well). Then every thing will be work correctly. I have such a patch works correctly.
> EXCEPT: SA migration, where SA lifetime comes from outside.
> I didn't look at SA migration part though, so any comments? Steffen
I have not looked into this for longer. So I can not comment on it
now, but I could be prepared for discussion on netdev01.
^ permalink raw reply
* Re: [PATCH net] ipv6: tcp: fix race in IPV6_2292PKTOPTIONS
From: David Miller @ 2015-01-26 8:44 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1421840742.4832.24.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Jan 2015 03:45:42 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> IPv6 TCP sockets store in np->pktoptions skbs, and use skb_set_owner_r()
> to charge the skb to socket.
>
> It means that destructor must be called while socket is locked.
>
> Therefore, we cannot use skb_get() or atomic_inc(&skb->users)
> to protect ourselves : kfree_skb() might race with other users
> manipulating sk->sk_forward_alloc
>
> Fix this race by holding socket lock for the duration of
> ip6_datagram_recv_ctl()
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> Since this bug is very old, feel free to apply on net-next ;)
Applied to net-next, thanks Eric.
^ permalink raw reply
* Re: ipv6: oops in datagram.c line 260
From: Steffen Klassert @ 2015-01-26 8:35 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Chris Ruehl, netdev, davem
In-Reply-To: <1420560073.32369.60.camel@redhat.com>
On Tue, Jan 06, 2015 at 05:01:13PM +0100, Hannes Frederic Sowa wrote:
> On Mi, 2014-12-24 at 21:42 +0800, Chris Ruehl wrote:
> > [447604.244357] ipv6_pinfo is NULL
> > [447604.273733] ------------[ cut here ]------------
> > [447604.303628] WARNING: CPU: 7 PID: 0 at net/ipv6/datagram.c:262
> > ipv6_local_error+0x16b/0x1a0()
> > [[...]]
> > [last unloaded: ipmi_si]
> > [447605.087999] CPU: 7 PID: 0 Comm: swapper/7 Not tainted 3.14.27 #11
> > [447605.139687] Hardware name: Dell Inc. PowerEdge R420/0CN7CM, BIOS 2.3.3
> > 07/10/2014
> > [447605.242931] 0000000000000009 ffff8806172e3b48 ffffffff815ffd58 0000000000000000
> > [447605.349130] ffff8806172e3b80 ffffffff81043c23 ffff8800a16322e8 ffff880037daa1c0
> > [447605.459659] ffff88000b026800 0000000000000000 ffff880037daa4b8 ffff8806172e3b90
> > [447605.576385] Call Trace:
> > [447605.634243] <IRQ> [<ffffffff815ffd58>] dump_stack+0x45/0x56
> > [447605.692870] [<ffffffff81043c23>] warn_slowpath_common+0x73/0x90
> > [447605.751097] [<ffffffff81043cf5>] warn_slowpath_null+0x15/0x20
> > [447605.808000] [<ffffffff815da6db>] ipv6_local_error+0x16b/0x1a0
> > [447605.863821] [<ffffffff815e29d0>] xfrm6_local_error+0x60/0x90
> > [447605.918493] [<ffffffff8150b485>] ? skb_dequeue+0x15/0x70
> > [447605.971871] [<ffffffff815a6cc1>] xfrm_local_error+0x51/0x70
> > [447606.024218] [<ffffffff8159ca15>] xfrm4_extract_output+0x75/0xb0
> > [447606.075630] [<ffffffff815a6c5a>] xfrm_inner_extract_output+0x6a/0x80
> > [447606.126055] [<ffffffff815e27a2>] xfrm6_prepare_output+0x12/0x60
> > [447606.175310] [<ffffffff815a6ed0>] xfrm_output_resume+0x1f0/0x370
> > [447606.223406] [<ffffffff8151a486>] ? skb_checksum_help+0x76/0x190
> > [447606.270572] [<ffffffff815a709b>] xfrm_output+0x3b/0xf0
> > [447606.316454] [<ffffffff815e2ae0>] ? xfrm6_extract_output+0xe0/0xe0
> > [447606.361803] [<ffffffff815e2af7>] xfrm6_output_finish+0x17/0x20
> > [447606.406053] [<ffffffff8159cad6>] xfrm4_output+0x46/0x80
> > [447606.448694] [<ffffffff81550a80>] ip_local_out+0x20/0x30
> > [447606.489952] [<ffffffff81550dd5>] ip_queue_xmit+0x135/0x3c0
> > [447606.530017] [<ffffffff815672e1>] tcp_transmit_skb+0x461/0x8c0
> > [447606.569362] [<ffffffff8156786e>] tcp_write_xmit+0x12e/0xb20
> > [447606.607876] [<ffffffff815669ff>] ? tcp_current_mss+0x4f/0x70
> > [447606.645723] [<ffffffff8156b320>] ? tcp_write_timer_handler+0x1b0/0x1b0
> > [447606.682837] [<ffffffff81569487>] tcp_send_loss_probe+0x37/0x1f0
> > [447606.719000] [<ffffffff8156b320>] ? tcp_write_timer_handler+0x1b0/0x1b0
> > [447606.754537] [<ffffffff8156b1bb>] tcp_write_timer_handler+0x4b/0x1b0
> > [447606.789266] [<ffffffff8156b320>] ? tcp_write_timer_handler+0x1b0/0x1b0
> > [447606.823242] [<ffffffff8156b378>] tcp_write_timer+0x58/0x60
> > [447606.856047] [<ffffffff8104e848>] call_timer_fn.isra.32+0x18/0x80
> > [447606.888029] [<ffffffff8104ea1a>] run_timer_softirq+0x16a/0x200
> > [447606.920224] [<ffffffff81047efc>] __do_softirq+0xec/0x250
> > [447606.951850] [<ffffffff810482f5>] irq_exit+0xf5/0x100
> > [447606.982665] [<ffffffff8102bc6f>] smp_apic_timer_interrupt+0x3f/0x50
> > [447607.014382] [<ffffffff8160d98a>] apic_timer_interrupt+0x6a/0x70
> > [447607.046175] <EOI> [<ffffffff8104f336>] ? get_next_timer_interrupt+0x1d6/0x250
> > [447607.111311] [<ffffffff814d45a7>] ? cpuidle_enter_state+0x47/0xc0
> > [447607.145850] [<ffffffff814d45a3>] ? cpuidle_enter_state+0x43/0xc0
> > [447607.179625] [<ffffffff814d46b6>] cpuidle_idle_call+0x96/0x130
> > [447607.213531] [<ffffffff8100b909>] arch_cpu_idle+0x9/0x20
> > [447607.247052] [<ffffffff810925ba>] cpu_startup_entry+0xda/0x1d0
> > [447607.280775] [<ffffffff81029d22>] start_secondary+0x212/0x2c0
> > [447607.314555] ---[ end trace 6ff3826b6e4fdf67 ]---
> >
>
> Thanks for the report!
>
> xfrm6_output_finish unconditionally resets skb->protocol so we try to
> dispatch to the IPv6 handler, even though tcp just sends an IPv4 packet.
>
Looks like we can postpone the setting of skb->protocol to the
xfrm{4,6}_prepare_output() functions where we finally switch to
outer mode.
This has two implications:
- We reset skb->protocol only for tunnel modes, should be ok.
- This affects the xfrm_output_gso() codepath on interfamily
tunnels. skb_mac_gso_segment() dispatches to the gso_segment()
callback functions via skb->protocol. So we dispatch to
the gso_segment() function of the outer mode what looks
wrong to me. If we postpone the setting of skb->protocol
to the xfrm{4,6}_prepare_output() we dispatch to inner mode
here.
Unfortunately I was not able to reproduce the problem on our test
setup. Chris could you try if the the patch below fixes your
problem?
Subject: [PATCH RFC] xfrm: Fix local error reporting crash with interfamily
tunnels
We set the outer mode protocol too early. As a result, the
local error handler might dispatch to the wrong address family
and report the error to a wrong socket type. We fix this by
seting the outer protocol to the skb after we accessed the
inner mode for the last time, right before we do the atcual
encapsulation where we switch finally to the outer mode.
Reported-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/xfrm4_output.c | 2 +-
net/ipv6/xfrm6_output.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index d5f6bd9..dab7381 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -63,6 +63,7 @@ int xfrm4_prepare_output(struct xfrm_state *x, struct sk_buff *skb)
return err;
IPCB(skb)->flags |= IPSKB_XFRM_TUNNEL_SIZE;
+ skb->protocol = htons(ETH_P_IP);
return x->outer_mode->output2(x, skb);
}
@@ -71,7 +72,6 @@ EXPORT_SYMBOL(xfrm4_prepare_output);
int xfrm4_output_finish(struct sk_buff *skb)
{
memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
- skb->protocol = htons(ETH_P_IP);
#ifdef CONFIG_NETFILTER
IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index ca3f29b..010f8bd 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -114,6 +114,7 @@ int xfrm6_prepare_output(struct xfrm_state *x, struct sk_buff *skb)
return err;
skb->ignore_df = 1;
+ skb->protocol = htons(ETH_P_IPV6);
return x->outer_mode->output2(x, skb);
}
@@ -122,7 +123,6 @@ EXPORT_SYMBOL(xfrm6_prepare_output);
int xfrm6_output_finish(struct sk_buff *skb)
{
memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
- skb->protocol = htons(ETH_P_IPV6);
#ifdef CONFIG_NETFILTER
IP6CB(skb)->flags |= IP6SKB_XFRM_TRANSFORMED;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver
From: Fugang Duan @ 2015-01-26 8:14 UTC (permalink / raw)
To: davem; +Cc: netdev, s.hauer, f.fainelli, bhutchings, stephen, b38611
The current .phy_detach() function only unbind generic phy driver, which causes
specifical driver suspend/resume function still work like Atheros AT803X PHYs.
For example:
ifconfig eth0 down
echo mem > /sys/power/status
After eth0 interface down, driver call phy_detach to unbind phy driver, and then
do suspend/resume operation, at803x_suspend()/at803x_resume() functions still get
called that call mdio bus read/write function. When eth0 interface down, MAC driver
may close all clocks and mdio bus cannot work. So the issue happens.
The patch can unbind generic and specifical driver.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
drivers/net/phy/phy_device.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 3fc91e8..8adbc5d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -672,8 +672,6 @@ EXPORT_SYMBOL(phy_attach);
*/
void phy_detach(struct phy_device *phydev)
{
- int i;
-
if (phydev->bus->dev.driver)
module_put(phydev->bus->dev.driver->owner);
@@ -681,17 +679,8 @@ void phy_detach(struct phy_device *phydev)
phydev->attached_dev = NULL;
phy_suspend(phydev);
- /* If the device had no specific driver before (i.e. - it
- * was using the generic driver), we unbind the device
- * from the generic driver so that there's a chance a
- * real driver could be loaded
- */
- for (i = 0; i < ARRAY_SIZE(genphy_driver); i++) {
- if (phydev->dev.driver == &genphy_driver[i].driver) {
- device_release_driver(&phydev->dev);
- break;
- }
- }
+ if (phydev->dev.driver)
+ device_release_driver(&phydev->dev);
}
EXPORT_SYMBOL(phy_detach);
--
1.7.8
^ permalink raw reply related
* Re: [net-next PATCH v3 00/12] Flow API
From: Simon Horman @ 2015-01-26 8:26 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: John Fastabend, Thomas Graf, Jiri Pirko, Pablo Neira Ayuso,
sfeldma, netdev, davem, gerlitz.or, andy, ast
In-Reply-To: <54C397C0.3010508@mojatatu.com>
On Sat, Jan 24, 2015 at 08:01:52AM -0500, Jamal Hadi Salim wrote:
[snip]
> Lets discuss at the meeting. I am just skimming these emails (the
> conference is chewing a lot of my time so i will mostly be absent).
> Sorry if i am not responding to some things.
Is "the meeting" the Hardware Offloading BoF at Netdev 1.0?
For the benefit of others: https://www.netdev01.org/sessions/10
^ permalink raw reply
* Re: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: Thomas Graf @ 2015-01-26 8:20 UTC (permalink / raw)
To: Herbert Xu; +Cc: Ying Xue, davem, kaber, paulmck, netdev, netfilter-devel
In-Reply-To: <E1YFWUl-0004gE-SH@gondolin.me.apana.org.au>
On 01/26/15 at 10:21am, Herbert Xu wrote:
> +int rhashtable_walk_start(struct rhashtable_iter *iter)
> +{
> + struct rhashtable *ht = iter->ht;
> + int err;
> +
> + err = mutex_lock_interruptible(&ht->mutex);
> + rcu_read_lock();
> +
> + if (!err)
> + mutex_unlock(&ht->mutex);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(rhashtable_walk_start);
This doesn't seem to work for Netlink dumps as it would define
a RCU read side section across user read()s.
I assume you relying on RCU to defer the initial table relinking
in the resizing to be postponed. Wouldn't it be better to hold
the mutex instead, we could sleep during the dump.
> +/**
> + * rhashtable_walk_next - Return the next object and advance the iterator
> + * @iter: Hash table iterator
> + *
> + * Note that you must call rhashtable_walk_stop when you are finished
> + * with the walk.
> + *
> + * Returns the next object or NULL when the end of the table is reached.
> + */
> +void *rhashtable_walk_next(struct rhashtable_iter *iter)
> +{
> + const struct bucket_table *tbl;
> + struct rhashtable *ht = iter->ht;
> + struct rhash_head *p = iter->p;
> +
> + tbl = rht_dereference_rcu(ht->tbl, ht);
> +
> + if (p) {
> + p = rht_dereference_bucket(p->next, tbl, iter->slot);
> + goto next;
> + }
> +
> + for (; iter->slot < tbl->size; iter->slot++) {
> + int skip = iter->skip;
> +
> + iter->lock = bucket_lock(tbl, iter->slot);
> + spin_lock_bh(iter->lock);
> +
> + rht_for_each(p, tbl, iter->slot) {
> + if (!skip)
> + break;
> + skip--;
> + }
> +
> +next:
> + if (!rht_is_a_nulls(p)) {
> + iter->skip++;
> + iter->p = p;
> + return rht_obj(ht, p);
Same here, we leave the iterator with the bucket lock held.
> + }
> + spin_unlock_bh(iter->lock);
> +
> + iter->skip = 0;
> + }
> +
> + iter->p = NULL;
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(rhashtable_walk_next);
^ permalink raw reply
* Re: [PATCH] rhashtable: fix rht_for_each_entry_safe() endless loop
From: David Miller @ 2015-01-26 8:08 UTC (permalink / raw)
To: kaber; +Cc: tgraf, netdev
In-Reply-To: <1421838733-16495-1-git-send-email-kaber@trash.net>
From: Patrick McHardy <kaber@trash.net>
Date: Wed, 21 Jan 2015 11:12:13 +0000
> "next" is not updated, causing an endless loop for buckets with more than
> one element.
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
Applied, thanks Patrick.
^ permalink raw reply
* [PATCH 1/2] xfrm6: Fix transport header offset in _decode_session6.
From: Steffen Klassert @ 2015-01-26 7:48 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1422258503-26978-1-git-send-email-steffen.klassert@secunet.com>
skb->transport_header might not be valid when we do a reverse
decode because the ipv6 tunnel error handlers don't update it
to the inner transport header. This leads to a wrong offset
calculation and to wrong layer 4 informations. We fix this
by using the size of the ipv6 header as the first offset.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/xfrm6_policy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 5f98364..aa48302 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -130,8 +130,8 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
{
struct flowi6 *fl6 = &fl->u.ip6;
int onlyproto = 0;
- u16 offset = skb_network_header_len(skb);
const struct ipv6hdr *hdr = ipv6_hdr(skb);
+ u16 offset = sizeof(*hdr);
struct ipv6_opt_hdr *exthdr;
const unsigned char *nh = skb_network_header(skb);
u8 nexthdr = nh[IP6CB(skb)->nhoff];
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] xfrm6: Fix the nexthdr offset in _decode_session6.
From: Steffen Klassert @ 2015-01-26 7:48 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1422258503-26978-1-git-send-email-steffen.klassert@secunet.com>
xfrm_decode_session() was originally designed for the
usage in the receive path where the correct nexthdr offset
is stored in IP6CB(skb)->nhoff. Over time this function
spread to code that is used in the output path (netfilter,
vti) where IP6CB(skb)->nhoff is not set. As a result, we
get a wrong nexthdr and the upper layer flow informations
are wrong. This can leed to incorrect policy lookups.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/xfrm6_policy.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index aa48302..48bf5a0 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -134,8 +134,14 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
u16 offset = sizeof(*hdr);
struct ipv6_opt_hdr *exthdr;
const unsigned char *nh = skb_network_header(skb);
- u8 nexthdr = nh[IP6CB(skb)->nhoff];
+ u16 nhoff = IP6CB(skb)->nhoff;
int oif = 0;
+ u8 nexthdr;
+
+ if (!nhoff)
+ nhoff = offsetof(struct ipv6hdr, nexthdr);
+
+ nexthdr = nh[nhoff];
if (skb_dst(skb))
oif = skb_dst(skb)->dev->ifindex;
--
1.9.1
^ permalink raw reply related
* pull request (net): ipsec 2015-01-26
From: Steffen Klassert @ 2015-01-26 7:48 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
Just two small fixes for _decode_session6() where we
might decode to wrong header information in some rare
situations.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit f2a01517f2a1040a0b156f171a7cefd748f2fd03:
openvswitch: Fix flow mask validation. (2014-12-05 21:42:16 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master
for you to fetch changes up to f855691975bb06373a98711e4cfe2c224244b536:
xfrm6: Fix the nexthdr offset in _decode_session6. (2014-12-08 07:56:18 +0100)
----------------------------------------------------------------
Steffen Klassert (2):
xfrm6: Fix transport header offset in _decode_session6.
xfrm6: Fix the nexthdr offset in _decode_session6.
net/ipv6/xfrm6_policy.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
^ permalink raw reply
* Re: [PATCH net 0/2] s390/qeth patches for net
From: David Miller @ 2015-01-26 7:38 UTC (permalink / raw)
To: ubraun; +Cc: netdev, linux-s390, ursula.braun
In-Reply-To: <1421843950-58220-1-git-send-email-ubraun@linux.vnet.ibm.com>
From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Wed, 21 Jan 2015 13:39:08 +0100
> here are two s390/qeth patches built for net.
> One patch is quite large, but we would like to fix the locking warning
> seen in recent kernels as soon as possible. But if you want me to submit
> these patches for net-next, I will do.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] net/fsl: Replace spin_event_timeout() with arch independent in xgmac_mdio
From: David Miller @ 2015-01-26 7:36 UTC (permalink / raw)
To: shh.xie; +Cc: netdev, Shaohui.Xie
In-Reply-To: <1421838512-826-1-git-send-email-shh.xie@gmail.com>
From: <shh.xie@gmail.com>
Date: Wed, 21 Jan 2015 19:08:32 +0800
> From: Shaohui Xie <Shaohui.Xie@freescale.com>
>
> spin_event_timeout() is PPC dependent, use an arch independent
> equivalent instead.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 1/2] net/fsl: drop in_be32() & out_be32() in xgmac_mdio
From: David Miller @ 2015-01-26 7:36 UTC (permalink / raw)
To: shh.xie; +Cc: netdev, Shaohui.Xie
In-Reply-To: <1421838469-782-1-git-send-email-shh.xie@gmail.com>
From: <shh.xie@gmail.com>
Date: Wed, 21 Jan 2015 19:07:49 +0800
> From: Shaohui Xie <Shaohui.Xie@freescale.com>
>
> Use ioread32be() & iowrite32be() instead.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH v2 net-next] bonding: handle more gso types
From: David Miller @ 2015-01-26 7:34 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, ogerlitz
In-Reply-To: <1422257234.29618.25.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 25 Jan 2015 23:27:14 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> In commit 5a7baa78851b ("bonding: Advertize vxlan offload features when
> supported"), Or Gerlitz added support conditional vxlan offload.
>
> In this patch I also add support for all kind of tunnels,
> but we allow a bonding device to not require segmentation,
> as it is always better to make this segmentation at the very last stage,
> if a particular slave device requires it.
>
> Tested:
>
> Setup a GRE tunnel,
> on a physical NIC not having tx-gre-segmentation.
> Results on bnx2x are even better, as we no longer have to segment
> in software.
>
> ethtool -K bond0 tx-gre-segmentation off
>
> super_netperf 50 --google-pacing-rate 30000000 -H 10.7.8.152 -l 15
> 7538.32
>
> ethtool -K bond0 tx-gre-segmentation on
>
> super_netperf 50 --google-pacing-rate 30000000 -H 10.7.8.152 -l 15
> 10200.5
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [patch] bridge: simplify br_getlink() a bit
From: David Miller @ 2015-01-26 7:33 UTC (permalink / raw)
To: dan.carpenter; +Cc: stephen, bridge, netdev, kernel-janitors
In-Reply-To: <20150121092235.GA19206@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 21 Jan 2015 12:22:35 +0300
> Static checkers complain that we should maybe set "ret" before we do the
> "goto out;". They interpret the NULL return from br_port_get_rtnl() as
> a failure and forgetting to set the error code is a common bug in this
> situation.
>
> The code is confusing but it's actually correct. We are returning zero
> deliberately. Let's re-write it a bit to be more clear.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied to net-next, thanks.
^ permalink raw reply
* [PATCH v2 net-next] bonding: handle more gso types
From: Eric Dumazet @ 2015-01-26 7:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Or Gerlitz
In-Reply-To: <1422021446.29618.8.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
In commit 5a7baa78851b ("bonding: Advertize vxlan offload features when
supported"), Or Gerlitz added support conditional vxlan offload.
In this patch I also add support for all kind of tunnels,
but we allow a bonding device to not require segmentation,
as it is always better to make this segmentation at the very last stage,
if a particular slave device requires it.
Tested:
Setup a GRE tunnel,
on a physical NIC not having tx-gre-segmentation.
Results on bnx2x are even better, as we no longer have to segment
in software.
ethtool -K bond0 tx-gre-segmentation off
super_netperf 50 --google-pacing-rate 30000000 -H 10.7.8.152 -l 15
7538.32
ethtool -K bond0 tx-gre-segmentation on
super_netperf 50 --google-pacing-rate 30000000 -H 10.7.8.152 -l 15
10200.5
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
---
v2: use NETIF_F_GSO_ENCAP_ALL instead of a private definition.
drivers/net/bonding/bond_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0dceba1a2ba15f4706922a5423f680e7cd17ef77..f47bc433407a176ed31a09e0010c87273c303a85 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -998,7 +998,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
NETIF_F_HIGHDMA | NETIF_F_LRO)
#define BOND_ENC_FEATURES (NETIF_F_ALL_CSUM | NETIF_F_SG | NETIF_F_RXCSUM |\
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL)
+ NETIF_F_TSO)
static void bond_compute_features(struct bonding *bond)
{
@@ -1034,7 +1034,7 @@ static void bond_compute_features(struct bonding *bond)
done:
bond_dev->vlan_features = vlan_features;
- bond_dev->hw_enc_features = enc_features;
+ bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL;
bond_dev->hard_header_len = max_hard_header_len;
bond_dev->gso_max_segs = gso_max_segs;
netif_set_gso_max_size(bond_dev, gso_max_size);
@@ -4010,7 +4010,7 @@ void bond_setup(struct net_device *bond_dev)
NETIF_F_HW_VLAN_CTAG_FILTER;
bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_HW_CSUM);
- bond_dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
+ bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
bond_dev->features |= bond_dev->hw_features;
}
^ permalink raw reply related
* RE: [PATCH net-next 1/7] r8152: adjust rx_bottom
From: Hayes Wang @ 2015-01-26 7:14 UTC (permalink / raw)
To: David Miller
Cc: sfeldma-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, nic_swsd,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150124.224342.425895080003524375.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
> Sent: Sunday, January 25, 2015 2:44 PM
[...]
> What keeps rtl_start_rx() from running in parallel with
> r8152_submit_rx(), or any other accessor of the RX agg->list?
Forgive my poor English. I would try to describe them clearly.
The steps about the rx agg->list would be
1. carrier on or autoresume occurs.
2. Call rtl_start_rx().
3. Rx agg->list flows between device and tp->rx_done.
4. carrier off or autosuspend occurs.
5. call rtl_stop_rx().
The rtl_start_rx() would only be called when the linking
status is changed from off to on or the auto resume occurs.
And rtl_start_rx() would reinitialize the tp->rx_done and
all of the rx agg->list. After step 2, the rx agg->list
would flow between the usb host controller and the driver.
If r8152_submit_rx() is success, the driver wouldn't own the
rx agg->list until it is returned from the usb host controller.
If r8152_submit_rx() is fail, the driver would still own the
rx agg->list, and queue it to the tp->rx_done with spin lock
for next try.
If the status stays in step 3, only the rx_bottom() would submit
the rx agg. The rtl_start_rx() wouldn't be called suddenly,
unless the linking down or auto suspend occur first and linking
on or auto resume occur again. If linking down or auto suspend
occur, rtl_stop_rx() would be called (step 5). After this step,
rx_bottom() wouldn't submit rx, and all rx agg->list would stop
flowing. That is, the tp->rx_done and all rx agg->list wouldn't
be changed until the next rtl_start_rx() is called.
Therefore, the flow for each rx agg->list would be
a. submittd by rtl_start_rx().
b. goto step c if success, otherwise goto step d.
c. completed by usb host controller.
d. queued to tp->rx_done with spin lock.
e. dequeue from tp->rx_done with spin lock by rx_botoom().
f. goto step i if link down, otherwise goto step g.
g. submitted by rx_botoom().
h. goto step b.
i. goto step a if link on.
And the patch change the step g to g1.
g1. submitted by rx_botoom() if (!ret), otherwise goto step d.
Best Regards,
Hayes
>
> You also keep using different terminology from me when
> discussing what lists do or do not need protection, and that
> is going to make it difficult for anyone to follow our
> conversation at all.
>
> We're talking specifically about RX agg->list objects and
> whether access to them need synchronization or not.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* [PATCH RFC] wlcore: match wait_for_completion_timeout return type
From: Nicholas Mc Guire @ 2015-01-26 6:26 UTC (permalink / raw)
To: Kalle Valo
Cc: Eliad Peller, Arik Nemtsov, Kobi L, John W. Linville,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Nicholas Mc Guire
return type of wait_for_completion_timeout is unsigned long not int, this
patch adds an appropriate return type and assignment.
Signed-off-by: Nicholas Mc Guire <der.herr-kA1LtwSENNE@public.gmane.org>
---
The return type of wait_for_completion_timeout is unsigned long not
int. This patch adds a separate variable of proper type for handling
of the wait_for_completion_timeout.
The alternative would be to fold it into the if condition and not use
a separate variable like so:
if (!pending) {
if (!wait_for_completion_timeout( &compl,
msecs_to_jiffies(WL1271_WAKEUP_TIMEOUT)) {
wl1271_error("ELP wakeup timeout!");
wl12xx_queue_recovery_work(wl);
ret = -ETIMEDOUT;
goto err;
}
}
not sure if this or the below solution is preferable.
Patch was compile tested only for x86_64_defconfig + CONFIG_WL_TI=y,
CONFIG_WLCORE=m
Patch is against 3.19.0-rc5 -next-20150123
drivers/net/wireless/ti/wlcore/ps.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/ps.c b/drivers/net/wireless/ti/wlcore/ps.c
index 4cd316e..d5f918a4 100644
--- a/drivers/net/wireless/ti/wlcore/ps.c
+++ b/drivers/net/wireless/ti/wlcore/ps.c
@@ -109,6 +109,7 @@ int wl1271_ps_elp_wakeup(struct wl1271 *wl)
DECLARE_COMPLETION_ONSTACK(compl);
unsigned long flags;
int ret;
+ unsigned long timeout;
unsigned long start_time = jiffies;
bool pending = false;
@@ -145,9 +146,9 @@ int wl1271_ps_elp_wakeup(struct wl1271 *wl)
}
if (!pending) {
- ret = wait_for_completion_timeout(
+ timeout = wait_for_completion_timeout(
&compl, msecs_to_jiffies(WL1271_WAKEUP_TIMEOUT));
- if (ret == 0) {
+ if (timeout == 0) {
wl1271_error("ELP wakeup timeout!");
wl12xx_queue_recovery_work(wl);
ret = -ETIMEDOUT;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 related
* [PATCH net-next v2 5/5] bonding: cleanup and remove dead code
From: Jonathan Toppins @ 2015-01-26 6:17 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek; +Cc: netdev
In-Reply-To: <1422253021-3798-1-git-send-email-jtoppins@cumulusnetworks.com>
fix sparse warning about non-static function
drivers/net/bonding/bond_main.c:3737:5: warning: symbol
'bond_3ad_xor_xmit' was not declared. Should it be static?
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
drivers/net/bonding/bond_main.c | 2 +-
include/net/bond_3ad.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c475d90..67437f3 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3734,7 +3734,7 @@ out:
* usable slave array is formed in the control path. The xmit function
* just calculates hash and sends the packet out.
*/
-int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
+static int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct bonding *bond = netdev_priv(dev);
struct slave *slave;
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index e01d903..f04cdbb 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -274,7 +274,6 @@ void bond_3ad_handle_link_change(struct slave *slave, char link);
int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
int __bond_3ad_get_active_agg_info(struct bonding *bond,
struct ad_info *ad_info);
-int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
struct slave *slave);
int bond_3ad_set_carrier(struct bonding *bond);
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next v2 4/5] bonding: fix LACP PDU not sent on slave port sometimes
From: Jonathan Toppins @ 2015-01-26 6:17 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
Cc: netdev, Satish Ashok, Andy Gospodarek
In-Reply-To: <1422253021-3798-1-git-send-email-jtoppins@cumulusnetworks.com>
From: Satish Ashok <sashok@cumulusnetworks.com>
When a slave is added to a bond and it is not in full duplex mode,
AD_PORT_LACP_ENABLED flag is cleared, due to this LACP PDU is not sent
on slave. When the duplex is changed to full, the flag needs to be set
to send LACP PDU.
Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Satish Ashok <sashok@cumulusnetworks.com>
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
drivers/net/bonding/bond_3ad.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index e3c96b2..cfc4a9c 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2219,8 +2219,10 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave,
switch (lacpdu->subtype) {
case AD_TYPE_LACPDU:
ret = RX_HANDLER_CONSUMED;
- netdev_dbg(slave->bond->dev, "Received LACPDU on port %d\n",
- port->actor_port_number);
+ netdev_dbg(slave->bond->dev,
+ "Received LACPDU on port %d slave %s\n",
+ port->actor_port_number,
+ slave->dev->name);
/* Protect against concurrent state machines */
spin_lock(&slave->bond->mode_lock);
ad_rx_machine(lacpdu, port);
@@ -2312,7 +2314,10 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
port->actor_admin_port_key &= ~AD_DUPLEX_KEY_MASKS;
port->actor_oper_port_key = port->actor_admin_port_key |=
__get_duplex(port);
- netdev_dbg(slave->bond->dev, "Port %d changed duplex\n", port->actor_port_number);
+ netdev_dbg(slave->bond->dev, "Port %d slave %s changed duplex\n",
+ port->actor_port_number, slave->dev->name);
+ if (port->actor_oper_port_key & AD_DUPLEX_KEY_MASKS)
+ port->sm_vars |= AD_PORT_LACP_ENABLED;
/* there is no need to reselect a new aggregator, just signal the
* state machines to reinitialize
*/
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next v2 3/5] bonding: fix incorrect lacp mux state when agg not active
From: Jonathan Toppins @ 2015-01-26 6:16 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
Cc: netdev, Wilson Kok, Andy Gospodarek
In-Reply-To: <1422253021-3798-1-git-send-email-jtoppins@cumulusnetworks.com>
From: Wilson Kok <wkok@cumulusnetworks.com>
This patch attempts to fix the following problems when an actor or
partner's aggregator is not active:
1. a slave's lacp port state is marked as AD_STATE_SYNCHRONIZATION
even if it is attached to an inactive aggregator. LACP advertises
this state to the partner, making the partner think he can move
into COLLECTING_DISTRIBUTING state even though this link will not
pass traffic on the local side
2. a slave goes into COLLECTING_DISTRIBUTING state without checking
if the aggregator is actually active
3. when in COLLECTING_DISTRIBUTING state, the partner parameters may
change, e.g. the partner_oper_port_state.SYNCHRONIZATION. The
local mux machine is not reacting to the change and continue to
keep the slave and bond up
4. When bond slave leaves an inactive aggregator and joins an active
aggregator, the actor oper port state need to update to SYNC state.
v2:
* fix style issues in bond_3ad.c
Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
drivers/net/bonding/bond_3ad.c | 44 +++++++++++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 8baa87d..e3c96b2 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -467,11 +467,14 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
/* set the partner sync. to on if the partner is sync,
* and the port is matched
*/
- if ((port->sm_vars & AD_PORT_MATCHED)
- && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION))
+ if ((port->sm_vars & AD_PORT_MATCHED) &&
+ (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION)) {
partner->port_state |= AD_STATE_SYNCHRONIZATION;
- else
+ pr_debug("%s partner sync=1\n", port->slave->dev->name);
+ } else {
partner->port_state &= ~AD_STATE_SYNCHRONIZATION;
+ pr_debug("%s partner sync=0\n", port->slave->dev->name);
+ }
}
}
@@ -726,6 +729,8 @@ static inline void __update_lacpdu_from_port(struct port *port)
lacpdu->actor_port_priority = htons(port->actor_port_priority);
lacpdu->actor_port = htons(port->actor_port_number);
lacpdu->actor_state = port->actor_oper_port_state;
+ pr_debug("update lacpdu: %s, actor port state %x\n",
+ port->slave->dev->name, port->actor_oper_port_state);
/* lacpdu->reserved_3_1 initialized
* lacpdu->tlv_type_partner_info initialized
@@ -898,7 +903,9 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
if ((port->sm_vars & AD_PORT_SELECTED) &&
(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) &&
!__check_agg_selection_timer(port)) {
- port->sm_mux_state = AD_MUX_COLLECTING_DISTRIBUTING;
+ if (port->aggregator->is_active)
+ port->sm_mux_state =
+ AD_MUX_COLLECTING_DISTRIBUTING;
} else if (!(port->sm_vars & AD_PORT_SELECTED) ||
(port->sm_vars & AD_PORT_STANDBY)) {
/* if UNSELECTED or STANDBY */
@@ -910,12 +917,16 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
*/
__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
port->sm_mux_state = AD_MUX_DETACHED;
+ } else if (port->aggregator->is_active) {
+ port->actor_oper_port_state |=
+ AD_STATE_SYNCHRONIZATION;
}
break;
case AD_MUX_COLLECTING_DISTRIBUTING:
if (!(port->sm_vars & AD_PORT_SELECTED) ||
(port->sm_vars & AD_PORT_STANDBY) ||
- !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION)) {
+ !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) ||
+ !(port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION)) {
port->sm_mux_state = AD_MUX_ATTACHED;
} else {
/* if port state hasn't changed make
@@ -937,8 +948,10 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
/* check if the state machine was changed */
if (port->sm_mux_state != last_state) {
- pr_debug("Mux Machine: Port=%d, Last State=%d, Curr State=%d\n",
- port->actor_port_number, last_state,
+ pr_debug("Mux Machine: Port=%d (%s), Last State=%d, Curr State=%d\n",
+ port->actor_port_number,
+ port->slave->dev->name,
+ last_state,
port->sm_mux_state);
switch (port->sm_mux_state) {
case AD_MUX_DETACHED:
@@ -953,7 +966,12 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
port->sm_mux_timer_counter = __ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
break;
case AD_MUX_ATTACHED:
- port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
+ if (port->aggregator->is_active)
+ port->actor_oper_port_state |=
+ AD_STATE_SYNCHRONIZATION;
+ else
+ port->actor_oper_port_state &=
+ ~AD_STATE_SYNCHRONIZATION;
port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
ad_disable_collecting_distributing(port,
@@ -963,6 +981,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
case AD_MUX_COLLECTING_DISTRIBUTING:
port->actor_oper_port_state |= AD_STATE_COLLECTING;
port->actor_oper_port_state |= AD_STATE_DISTRIBUTING;
+ port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
ad_enable_collecting_distributing(port,
update_slave_arr);
port->ntt = true;
@@ -1044,8 +1063,10 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
/* check if the State machine was changed or new lacpdu arrived */
if ((port->sm_rx_state != last_state) || (lacpdu)) {
- pr_debug("Rx Machine: Port=%d, Last State=%d, Curr State=%d\n",
- port->actor_port_number, last_state,
+ pr_debug("Rx Machine: Port=%d (%s), Last State=%d, Curr State=%d\n",
+ port->actor_port_number,
+ port->slave->dev->name,
+ last_state,
port->sm_rx_state);
switch (port->sm_rx_state) {
case AD_RX_INITIALIZE:
@@ -1394,6 +1415,9 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
aggregator = __get_first_agg(port);
ad_agg_selection_logic(aggregator, update_slave_arr);
+
+ if (!port->aggregator->is_active)
+ port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
}
/* Decide if "agg" is a better choice for the new active aggregator that
--
1.7.10.4
^ permalink raw reply related
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