* Re: [PATCH net-next v1 0/6] bonding: remove bond->vlan_list
From: Veaceslav Falico @ 2013-08-08 10:39 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Andy Gospodarek, Patrick McHardy, David S. Miller,
Nikolay Aleksandrov
In-Reply-To: <1375957269-13100-1-git-send-email-vfalico@redhat.com>
On Thu, Aug 08, 2013 at 12:21:03PM +0200, Veaceslav Falico wrote:
>RFC -> v1: Got some feedback from Nikolay Aleksandrov (privately), tried to
> address it, also fixed some bugs that I've found on the way. I
> think it's ready to be considered a patchset for
> review/inclusion in net-next.
I've re-sent the 1/6 (with v2), got the old version from the wrong git
branch.
>
>The aim of this patchset is to remove bond->vlan_list completely, and use
>8021q's standard functions instead of it.
>
>The patchset is on top of Nik's latest two patches:
>[net-next,v2,1/2] bonding: change the bond's vlan syncing functions with
> the standard ones
>[net-next,v2,2/2] bonding: unwind on bond_add_vlan failure
>
>
>First two patches add two helper functions to vlan code:
>
>bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
>
> Here we change vlan_uses_dev() to be able to work under both rtnl
> and rcu, and use it under rcu_read_lock() in bond_vlan_used().
>
>vlan: add __vlan_find_dev_next()
>
> This function takes dev and vlan_dev and returns the next vlan dev
> that uses this dev. It can be used to cycle through the vlan list,
> and not only by bonding - but by any network driver that uses its
> private vlan list.
>
>Next four patches actually convert bonding to use the new
>functions/approach and remove the vlan_list completely.
>
>This patchset solves several issues with bonding, simplify it overall,
>RCUify further and add infrastructure to anyone else who'd like to use
>8021q standard functions instead of their own vlan_list, which is quite
>common amongst network drivers currently.
>
>I'm testing it continuously currently, no issues found, will update on
>anything.
>
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>CC: Patrick McHardy <kaber@trash.net>
>CC: "David S. Miller" <davem@davemloft.net>
>CC: Nikolay Aleksandrov <nikolay@redhat.com>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>
>---
> drivers/net/bonding/bond_alb.c | 48 ++++++++----
> drivers/net/bonding/bond_alb.h | 2 +-
> drivers/net/bonding/bond_main.c | 163 ++++++---------------------------------
> drivers/net/bonding/bonding.h | 16 ++--
> include/linux/if_vlan.h | 8 ++
> net/8021q/vlan.h | 6 +-
> net/8021q/vlan_core.c | 36 ++++++++-
> 7 files changed, 115 insertions(+), 164 deletions(-)
^ permalink raw reply
* Re: [PATCH v3 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac
From: Prabhakar Lad @ 2013-08-08 10:39 UTC (permalink / raw)
To: Sekhar Nori
Cc: DLOS, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, LKML, Heiko Schocher,
LAK
In-Reply-To: <5203C1D1.8020707-l0cyMroinI0@public.gmane.org>
On Thu, Aug 8, 2013 at 9:35 PM, Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
> On Thursday 08 August 2013 04:02 PM, Prabhakar Lad wrote:
>> Hi Sekhar,
>>
>> Sorry for the delayed response I was on leave and now back again up
>> and running.
>>
>> On Wed, Jul 31, 2013 at 11:17 AM, Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
>>> On Sunday 23 June 2013 08:30 PM, Prabhakar Lad wrote:
>>>> From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> This patch makes a common function for to configure emac and calls
>>>> it appropriately in DT and non DT boot mode. The system configuration
>>>> chip CFGCHIP3, controls the emac module. This patch appropriately
>>>> configures this register for emac and sets DA850_MII_MDIO_CLKEN_PIN
>>>> GPIO pin appropriately.
>>>>
>> [Snip]
>>
>>>> + if (rmii_enabled)
>>>> + val |= BIT(8);
>>>> + else
>>>> + val &= ~BIT(8);
>>>> +
>>>> + /* configure the CFGCHIP3 register for RMII or MII */
>>>> + writel(val, cfg_chip3_base);
>>>> +
>>>> + ret = davinci_cfg_reg(DA850_GPIO2_6);
>>>> + if (ret)
>>>> + pr_warn("%s:GPIO(2,6) mux setup failed\n", __func__);
>>>> +
>>>> + ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
>>>> + if (ret) {
>>>> + pr_warn("Cannot open GPIO %d\n", DA850_MII_MDIO_CLKEN_PIN);
>>>> + return;
>>>> + }
>>>
>>> You cannot do this in SoC specific code. All boards wont use a GPIO to
>>> choose between MII/RMII. You need to do this in a EVM specific manner.
>>> This just means you retain the GPIO part of code in board-da850-evm.c
>>
>> OK
>>
>>> and for the DT case use a board specific GPIO node. See a similar
>>> example in arch/arm/boot/dts/ste-nomadik-s8815.dts and how the code uses
>> Alrite something similar to usb-s8815 node.
>>
>>> it in arch/arm/mach-nomadik/cpu-8815.c.
>>>
>> But I don't see any code to access this node in this file, can
>> you point me to right direction ?
>
> Look at cpu8815_eth_init() function in this file.
>
Thanks, I was looking at some earlier version of code here [1],
where this was missing.
[1] https://github.com/AndrewDB/rk3066-kernel/blob/master/arch/arm/mach-nomadik/cpu-8815.c
Regards,
--Prabhakar Lad
^ permalink raw reply
* [PATCH v2 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
From: Veaceslav Falico @ 2013-08-08 10:39 UTC (permalink / raw)
To: netdev
Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek, Patrick McHardy,
David S. Miller, Nikolay Aleksandrov
In-Reply-To: <1375957269-13100-2-git-send-email-vfalico@redhat.com>
RFC -> v1: don't add vlan_uses_dev_rcu() and change vlan_uses_dev() instead
v1 -> v2: remove the ASSERT_RTNL(), cause we can now be called under rcu.
Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
id 0, and always returns true if 8021q is loaded. This creates several bad
situations - some warnings in __bond_release_one() because it thinks that
we still have vlans while removing, sending LB packets with vlan id 0 and,
possibly, other caused by vlan id 0.
Fix it by changing vlan_uses_dev() to use rcu_dereference_rtnl() instead of
rtnl_dereference(), and thus it can already be used in bond_vlan_used()
under rcu_read_lock().
By the time vlan_uses_dev() returns we cannot be sure if there were no
vlans added/removed, so it's basicly an optimization function.
Also, use the vlan_uses_dev() in __bond_release_one() cause the rtnl lock
is held there.
For this call to be visible in bonding.h, add include <linux/if_vlan.h>,
and also remove it from any other bonding file, cause they all include
bonding.h, and thus linux/if_vlan.h.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_alb.c | 1 -
drivers/net/bonding/bond_main.c | 3 +--
drivers/net/bonding/bonding.h | 10 +++++++++-
net/8021q/vlan_core.c | 5 ++---
4 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 3a5db7b..2684329 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -34,7 +34,6 @@
#include <linux/if_arp.h>
#include <linux/if_ether.h>
#include <linux/if_bonding.h>
-#include <linux/if_vlan.h>
#include <linux/in.h>
#include <net/ipx.h>
#include <net/arp.h>
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 4264a76..9d1045d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -69,7 +69,6 @@
#include <net/arp.h>
#include <linux/mii.h>
#include <linux/ethtool.h>
-#include <linux/if_vlan.h>
#include <linux/if_bonding.h>
#include <linux/jiffies.h>
#include <linux/preempt.h>
@@ -1953,7 +1952,7 @@ static int __bond_release_one(struct net_device *bond_dev,
bond_set_carrier(bond);
eth_hw_addr_random(bond_dev);
- if (bond_vlan_used(bond)) {
+ if (vlan_uses_dev(bond_dev)) {
pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n",
bond_dev->name, bond_dev->name);
pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n",
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 4bf52d5..9c4539e 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -23,6 +23,7 @@
#include <linux/netpoll.h>
#include <linux/inetdevice.h>
#include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
#include "bond_3ad.h"
#include "bond_alb.h"
@@ -267,9 +268,16 @@ struct bonding {
#endif /* CONFIG_DEBUG_FS */
};
+/* use vlan_uses_dev() if under rtnl */
static inline bool bond_vlan_used(struct bonding *bond)
{
- return !list_empty(&bond->vlan_list);
+ bool ret;
+
+ rcu_read_lock();
+ ret = vlan_uses_dev(bond->dev);
+ rcu_read_unlock();
+
+ return ret;
}
#define bond_slave_get_rcu(dev) \
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 4a78c4d..361c97b 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -387,13 +387,12 @@ void vlan_vids_del_by_dev(struct net_device *dev,
}
EXPORT_SYMBOL(vlan_vids_del_by_dev);
+/* Must hold either rtnl or rcu_read_lock */
bool vlan_uses_dev(const struct net_device *dev)
{
struct vlan_info *vlan_info;
- ASSERT_RTNL();
-
- vlan_info = rtnl_dereference(dev->vlan_info);
+ vlan_info = rcu_dereference_rtnl(dev->vlan_info);
if (!vlan_info)
return false;
return vlan_info->grp.nr_vlan_devs ? true : false;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v3 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac
From: Prabhakar Lad @ 2013-08-08 10:32 UTC (permalink / raw)
To: Sekhar Nori; +Cc: DLOS, LAK, devicetree-discuss, LKML, netdev, Heiko Schocher
In-Reply-To: <51F8A4FF.8020803@ti.com>
Hi Sekhar,
Sorry for the delayed response I was on leave and now back again up
and running.
On Wed, Jul 31, 2013 at 11:17 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Sunday 23 June 2013 08:30 PM, Prabhakar Lad wrote:
>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>
>> This patch makes a common function for to configure emac and calls
>> it appropriately in DT and non DT boot mode. The system configuration
>> chip CFGCHIP3, controls the emac module. This patch appropriately
>> configures this register for emac and sets DA850_MII_MDIO_CLKEN_PIN
>> GPIO pin appropriately.
>>
[Snip]
>> + if (rmii_enabled)
>> + val |= BIT(8);
>> + else
>> + val &= ~BIT(8);
>> +
>> + /* configure the CFGCHIP3 register for RMII or MII */
>> + writel(val, cfg_chip3_base);
>> +
>> + ret = davinci_cfg_reg(DA850_GPIO2_6);
>> + if (ret)
>> + pr_warn("%s:GPIO(2,6) mux setup failed\n", __func__);
>> +
>> + ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
>> + if (ret) {
>> + pr_warn("Cannot open GPIO %d\n", DA850_MII_MDIO_CLKEN_PIN);
>> + return;
>> + }
>
> You cannot do this in SoC specific code. All boards wont use a GPIO to
> choose between MII/RMII. You need to do this in a EVM specific manner.
> This just means you retain the GPIO part of code in board-da850-evm.c
OK
> and for the DT case use a board specific GPIO node. See a similar
> example in arch/arm/boot/dts/ste-nomadik-s8815.dts and how the code uses
Alrite something similar to usb-s8815 node.
> it in arch/arm/mach-nomadik/cpu-8815.c.
>
But I don't see any code to access this node in this file, can
you point me to right direction ?
Regards,
--Prabhakar Lad
^ permalink raw reply
* [PATCH v1 net-next 6/6] bonding: remove unused bond->vlan_list
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1375957269-13100-1-git-send-email-vfalico@redhat.com>
RFC -> v1: No changes.
There are currently no users of bond->vlan_list (other than the maintaining
functions add/remove) - so remove it and every unneeded helper.
In this patch we remove:
vlan_list from struct bonding
bond_next_vlan - we don't need it anymore
struct vlan_entry - it was a helper struct for bond->vlan_list
some bits from bond_vlan_rx_add/kill_vid() - which were related to
bond->vlan_list
(de)initialization of vlan_list from bond_setup/uninit
bond_add_vlan - its only scope was to maintain bond->vlan_list
We don't fully remove bond_del_vlan() - bond_alb_clear_vlan() still needs
to be called when a vlan disappears. And we make bond_del_vlan() to not
return anything cause it cannot fail already.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 117 +-------------------------------------
drivers/net/bonding/bonding.h | 6 --
2 files changed, 4 insertions(+), 119 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 215c497..49bdda2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -282,113 +282,24 @@ const char *bond_mode_name(int mode)
/*---------------------------------- VLAN -----------------------------------*/
/**
- * bond_add_vlan - add a new vlan id on bond
- * @bond: bond that got the notification
- * @vlan_id: the vlan id to add
- *
- * Returns -ENOMEM if allocation failed.
- */
-static int bond_add_vlan(struct bonding *bond, unsigned short vlan_id)
-{
- struct vlan_entry *vlan;
-
- pr_debug("bond: %s, vlan id %d\n",
- (bond ? bond->dev->name : "None"), vlan_id);
-
- vlan = kzalloc(sizeof(struct vlan_entry), GFP_KERNEL);
- if (!vlan)
- return -ENOMEM;
-
- INIT_LIST_HEAD(&vlan->vlan_list);
- vlan->vlan_id = vlan_id;
-
- write_lock_bh(&bond->lock);
-
- list_add_tail(&vlan->vlan_list, &bond->vlan_list);
-
- write_unlock_bh(&bond->lock);
-
- pr_debug("added VLAN ID %d on bond %s\n", vlan_id, bond->dev->name);
-
- return 0;
-}
-
-/**
* bond_del_vlan - delete a vlan id from bond
* @bond: bond that got the notification
* @vlan_id: the vlan id to delete
*
* returns -ENODEV if @vlan_id was not found in @bond.
*/
-static int bond_del_vlan(struct bonding *bond, unsigned short vlan_id)
+static void bond_del_vlan(struct bonding *bond, unsigned short vlan_id)
{
- struct vlan_entry *vlan;
- int res = -ENODEV;
-
pr_debug("bond: %s, vlan id %d\n", bond->dev->name, vlan_id);
block_netpoll_tx();
write_lock_bh(&bond->lock);
- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- if (vlan->vlan_id == vlan_id) {
- list_del(&vlan->vlan_list);
-
- if (bond_is_lb(bond))
- bond_alb_clear_vlan(bond, vlan_id);
-
- pr_debug("removed VLAN ID %d from bond %s\n",
- vlan_id, bond->dev->name);
+ if (bond_is_lb(bond))
+ bond_alb_clear_vlan(bond, vlan_id);
- kfree(vlan);
-
- res = 0;
- goto out;
- }
- }
-
- pr_debug("couldn't find VLAN ID %d in bond %s\n",
- vlan_id, bond->dev->name);
-
-out:
write_unlock_bh(&bond->lock);
unblock_netpoll_tx();
- return res;
-}
-
-/**
- * bond_next_vlan - safely skip to the next item in the vlans list.
- * @bond: the bond we're working on
- * @curr: item we're advancing from
- *
- * Returns %NULL if list is empty, bond->next_vlan if @curr is %NULL,
- * or @curr->next otherwise (even if it is @curr itself again).
- *
- * Caller must hold bond->lock
- */
-struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr)
-{
- struct vlan_entry *next, *last;
-
- if (list_empty(&bond->vlan_list))
- return NULL;
-
- if (!curr) {
- next = list_entry(bond->vlan_list.next,
- struct vlan_entry, vlan_list);
- } else {
- last = list_entry(bond->vlan_list.prev,
- struct vlan_entry, vlan_list);
- if (last == curr) {
- next = list_entry(bond->vlan_list.next,
- struct vlan_entry, vlan_list);
- } else {
- next = list_entry(curr->vlan_list.next,
- struct vlan_entry, vlan_list);
- }
- }
-
- return next;
}
/**
@@ -450,13 +361,6 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
goto unwind;
}
- res = bond_add_vlan(bond, vid);
- if (res) {
- pr_err("%s: Error: Failed to add vlan id %d\n",
- bond_dev->name, vid);
- goto unwind;
- }
-
return 0;
unwind:
@@ -477,17 +381,11 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
{
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave;
- int res;
bond_for_each_slave(bond, slave)
vlan_vid_del(slave->dev, proto, vid);
- res = bond_del_vlan(bond, vid);
- if (res) {
- pr_err("%s: Error: Failed to remove vlan id %d\n",
- bond_dev->name, vid);
- return res;
- }
+ bond_del_vlan(bond, vid);
return 0;
}
@@ -4135,7 +4033,6 @@ static void bond_setup(struct net_device *bond_dev)
/* Initialize pointers */
bond->dev = bond_dev;
- INIT_LIST_HEAD(&bond->vlan_list);
/* Initialize the device entry points */
ether_setup(bond_dev);
@@ -4188,7 +4085,6 @@ static void bond_uninit(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave, *tmp_slave;
- struct vlan_entry *vlan, *tmp;
bond_netpoll_cleanup(bond_dev);
@@ -4200,11 +4096,6 @@ static void bond_uninit(struct net_device *bond_dev)
list_del(&bond->bond_list);
bond_debug_unregister(bond);
-
- list_for_each_entry_safe(vlan, tmp, &bond->vlan_list, vlan_list) {
- list_del(&vlan->vlan_list);
- kfree(vlan);
- }
}
/*------------------------- Module initialization ---------------------------*/
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 9c4539e..bfa33c4 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -186,11 +186,6 @@ struct bond_parm_tbl {
#define BOND_MAX_MODENAME_LEN 20
-struct vlan_entry {
- struct list_head vlan_list;
- unsigned short vlan_id;
-};
-
struct slave {
struct net_device *dev; /* first - useful for panic debug */
struct list_head list;
@@ -255,7 +250,6 @@ struct bonding {
struct ad_bond_info ad_info;
struct alb_bond_info alb_info;
struct bond_params params;
- struct list_head vlan_list;
struct workqueue_struct *wq;
struct delayed_work mii_work;
struct delayed_work arp_work;
--
1.7.1
^ permalink raw reply related
* [PATCH v1 net-next 5/6] bonding: convert bond_arp_send_all to use bond->dev->vlan_info
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1375957269-13100-1-git-send-email-vfalico@redhat.com>
RFC -> v1: use the new __vlan_find_dev_next(), also release rcu_read_lock()
only after we stop using the vlan_dev.
Instead of looping through bond->vlan_list, loop through
bond->dev->vlan_info via __vlan_find_dev_next() under rcu_read_lock().
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 29 +++++++++++++----------------
1 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 14447a6..215c497 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2440,11 +2440,10 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, __be32 dest_
static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
{
- int i, vlan_id;
- __be32 *targets = bond->params.arp_targets;
- struct vlan_entry *vlan;
- struct net_device *vlan_dev = NULL;
+ struct net_device *vlan_dev;
struct rtable *rt;
+ __be32 *targets = bond->params.arp_targets;
+ int i;
for (i = 0; (i < BOND_MAX_ARP_TARGETS); i++) {
__be32 addr;
@@ -2486,28 +2485,26 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
continue;
}
- vlan_id = 0;
- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- rcu_read_lock();
- vlan_dev = __vlan_find_dev_deep(bond->dev,
- htons(ETH_P_8021Q),
- vlan->vlan_id);
- rcu_read_unlock();
+ vlan_dev = NULL;
+
+ rcu_read_lock();
+ while ((vlan_dev = __vlan_find_dev_next(bond->dev, vlan_dev)))
if (vlan_dev == rt->dst.dev) {
- vlan_id = vlan->vlan_id;
pr_debug("basa: vlan match on %s %d\n",
- vlan_dev->name, vlan_id);
+ vlan_dev->name,
+ vlan_dev_vlan_id(vlan_dev));
break;
}
- }
- if (vlan_id && vlan_dev) {
+ if (vlan_dev) {
ip_rt_put(rt);
addr = bond_confirm_addr(vlan_dev, targets[i], 0);
bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
- addr, vlan_id);
+ addr, vlan_dev_vlan_id(vlan_dev));
+ rcu_read_unlock();
continue;
}
+ rcu_read_unlock();
if (net_ratelimit()) {
pr_warning("%s: no path to arp_ip_target %pI4 via rt.dev %s\n",
--
1.7.1
^ permalink raw reply related
* [PATCH v1 net-next 4/6] bonding: convert bond_has_this_ip to use bond->dev->vlan_info
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1375957269-13100-1-git-send-email-vfalico@redhat.com>
RFC -> v1: use the new __vlan_find_dev_next(), which simplifies the code
and omits issues with vlan id 0.
Use __vlan_find_dev_next() to loop through dev's vlan devices and verify if
the ip matches.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9d1045d..14447a6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2392,20 +2392,18 @@ re_arm:
static int bond_has_this_ip(struct bonding *bond, __be32 ip)
{
- struct vlan_entry *vlan;
struct net_device *vlan_dev;
if (ip == bond_confirm_addr(bond->dev, 0, ip))
return 1;
- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- rcu_read_lock();
- vlan_dev = __vlan_find_dev_deep(bond->dev, htons(ETH_P_8021Q),
- vlan->vlan_id);
- rcu_read_unlock();
- if (vlan_dev && ip == bond_confirm_addr(vlan_dev, 0, ip))
+ rcu_read_lock();
+ while ((vlan_dev = __vlan_find_dev_next(bond->dev, vlan_dev)))
+ if (ip == bond_confirm_addr(vlan_dev, 0, ip)) {
+ rcu_read_unlock();
return 1;
- }
+ }
+ rcu_read_unlock();
return 0;
}
--
1.7.1
^ permalink raw reply related
* [PATCH v1 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev
Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek, Patrick McHardy,
David S. Miller, Nikolay Aleksandrov
In-Reply-To: <1375957269-13100-1-git-send-email-vfalico@redhat.com>
RFC -> v1: don't add vlan_uses_dev_rcu() and change vlan_uses_dev() instead
Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
id 0, and always returns true if 8021q is loaded. This creates several bad
situations - some warnings in __bond_release_one() because it thinks that
we still have vlans while removing, sending LB packets with vlan id 0 and,
possibly, other caused by vlan id 0.
Fix it by changing vlan_uses_dev() to use rcu_dereference_rtnl() instead of
rtnl_dereference(), and thus it can already be used in bond_vlan_used()
under rcu_read_lock().
By the time vlan_uses_dev() returns we cannot be sure if there were no
vlans added/removed, so it's basicly an optimization function.
Also, use the vlan_uses_dev() in __bond_release_one() cause the rtnl lock
is held there.
For this call to be visible in bonding.h, add include <linux/if_vlan.h>,
and also remove it from any other bonding file, cause they all include
bonding.h, and thus linux/if_vlan.h.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_alb.c | 1 -
drivers/net/bonding/bond_main.c | 3 +--
drivers/net/bonding/bonding.h | 10 +++++++++-
net/8021q/vlan_core.c | 3 ++-
4 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 3a5db7b..2684329 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -34,7 +34,6 @@
#include <linux/if_arp.h>
#include <linux/if_ether.h>
#include <linux/if_bonding.h>
-#include <linux/if_vlan.h>
#include <linux/in.h>
#include <net/ipx.h>
#include <net/arp.h>
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 4264a76..9d1045d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -69,7 +69,6 @@
#include <net/arp.h>
#include <linux/mii.h>
#include <linux/ethtool.h>
-#include <linux/if_vlan.h>
#include <linux/if_bonding.h>
#include <linux/jiffies.h>
#include <linux/preempt.h>
@@ -1953,7 +1952,7 @@ static int __bond_release_one(struct net_device *bond_dev,
bond_set_carrier(bond);
eth_hw_addr_random(bond_dev);
- if (bond_vlan_used(bond)) {
+ if (vlan_uses_dev(bond_dev)) {
pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n",
bond_dev->name, bond_dev->name);
pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n",
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 4bf52d5..9c4539e 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -23,6 +23,7 @@
#include <linux/netpoll.h>
#include <linux/inetdevice.h>
#include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
#include "bond_3ad.h"
#include "bond_alb.h"
@@ -267,9 +268,16 @@ struct bonding {
#endif /* CONFIG_DEBUG_FS */
};
+/* use vlan_uses_dev() if under rtnl */
static inline bool bond_vlan_used(struct bonding *bond)
{
- return !list_empty(&bond->vlan_list);
+ bool ret;
+
+ rcu_read_lock();
+ ret = vlan_uses_dev(bond->dev);
+ rcu_read_unlock();
+
+ return ret;
}
#define bond_slave_get_rcu(dev) \
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 4a78c4d..12e1606 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -387,13 +387,14 @@ void vlan_vids_del_by_dev(struct net_device *dev,
}
EXPORT_SYMBOL(vlan_vids_del_by_dev);
+/* Must hold either rtnl or rcu_read_lock */
bool vlan_uses_dev(const struct net_device *dev)
{
struct vlan_info *vlan_info;
ASSERT_RTNL();
- vlan_info = rtnl_dereference(dev->vlan_info);
+ vlan_info = rcu_dereference_rtnl(dev->vlan_info);
if (!vlan_info)
return false;
return vlan_info->grp.nr_vlan_devs ? true : false;
--
1.7.1
^ permalink raw reply related
* [PATCH v1 net-next 3/6] bonding: make bond_alb use 8021q's dev->vlan_info instead of vlan_list
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1375957269-13100-1-git-send-email-vfalico@redhat.com>
RFC -> v1: use the changed __vlan_find_dev_next, which now works with
vlan's net_device instead of vlan's id. Also, fix a subtle race
condition if we remove the only vlan while looping through
MAX_LP_BURST - we end up with using the old vlan_id, so set it
to 0 if we don't have vlans.
In alb mode, we only need each vlan's id (that is on top of bond) to tag
learning packets, so get them via __vlan_find_dev_next(bond->dev, last_dev).
We must also find *any* vlan (including last id stored in current_alb_vlan)
if we can't find anything >= current_alb_vlan id.
For that, we verify if bond has any vlans at all, and if yes - find the
next vlan id after current_alb_vlan id. So, if vlan id is not 0, we tag the
skb.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_alb.c | 47 ++++++++++++++++++++++++++++-----------
drivers/net/bonding/bond_alb.h | 2 +-
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 2684329..ced5753 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -974,8 +974,9 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
{
struct bonding *bond = bond_get_bond_by_slave(slave);
struct learning_pkt pkt;
+ struct net_device *vlan_dev;
int size = sizeof(struct learning_pkt);
- int i;
+ int i, vlan_id;
memset(&pkt, 0, size);
memcpy(pkt.mac_dst, mac_addr, ETH_ALEN);
@@ -1000,22 +1001,42 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
skb->priority = TC_PRIO_CONTROL;
skb->dev = slave->dev;
+ rcu_read_lock();
if (bond_vlan_used(bond)) {
- struct vlan_entry *vlan;
-
- vlan = bond_next_vlan(bond,
- bond->alb_info.current_alb_vlan);
-
- bond->alb_info.current_alb_vlan = vlan;
- if (!vlan) {
+ /* first try to find the previously used vlan by
+ * id, which might have gone away already
+ */
+ vlan_id = bond->alb_info.current_alb_vlan;
+ vlan_dev = __vlan_find_dev_deep(bond->dev,
+ htons(ETH_P_8021Q),
+ vlan_id);
+
+ /* search for the next one, if not found - for any */
+ if (vlan_dev)
+ vlan_dev = __vlan_find_dev_next(bond->dev,
+ vlan_dev);
+ if (!vlan_dev)
+ vlan_dev = __vlan_find_dev_next(bond->dev,
+ NULL);
+
+ if (vlan_dev) {
+ vlan_id = vlan_dev_vlan_id(vlan_dev);
+ bond->alb_info.current_alb_vlan = vlan_id;
+ } else {
+ bond->alb_info.current_alb_vlan = 0;
+ rcu_read_unlock();
kfree_skb(skb);
continue;
}
+ } else
+ vlan_id = 0;
+ rcu_read_unlock();
- skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vlan->vlan_id);
+ if (vlan_id) {
+ skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_id);
if (!skb) {
- pr_err("%s: Error: failed to insert VLAN tag\n",
- bond->dev->name);
+ pr_err("%s: Error: failed to insert VLAN tag %d\n",
+ bond->dev->name, vlan_id);
continue;
}
}
@@ -1759,8 +1780,8 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
void bond_alb_clear_vlan(struct bonding *bond, unsigned short vlan_id)
{
if (bond->alb_info.current_alb_vlan &&
- (bond->alb_info.current_alb_vlan->vlan_id == vlan_id)) {
- bond->alb_info.current_alb_vlan = NULL;
+ (bond->alb_info.current_alb_vlan == vlan_id)) {
+ bond->alb_info.current_alb_vlan = 0;
}
if (bond->alb_info.rlb_enabled) {
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index e7a5b8b..b2452aa 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -170,7 +170,7 @@ struct alb_bond_info {
* rx traffic should be
* rebalanced
*/
- struct vlan_entry *current_alb_vlan;
+ u16 current_alb_vlan;
};
int bond_alb_initialize(struct bonding *bond, int rlb_enabled);
--
1.7.1
^ permalink raw reply related
* [PATCH v1 net-next 2/6] vlan: add __vlan_find_dev_next()
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Patrick McHardy, David S. Miller
In-Reply-To: <1375957269-13100-1-git-send-email-vfalico@redhat.com>
RFC -> v1: make the function accept/return vlan's net_device, this way we
won't have troubles with VLAN 0, and the user code will be
cleaner and faster.
Add a new exported function __vlan_find_dev_next(dev, vlan_dev), which
returns the a vlan's net_device that is used by the dev and is its id is
greater or equal to vlan_dev's vlan id. If vlan_dev is NULL, return first
vlan, if nothing is found return NULL.
This function must be under rcu_read_lock(), is aware of master devices and
doesn't guarantee that, once it returns, the vlan dev will still be used by
dev as its vlan.
It's basically a helper for "for_each_vlan_in_dev(dev, vlan_dev)" logic,
and is supposed to be used like this:
vlan_dev = NULL;
while ((vlan_dev = __vlan_find_dev_next(dev, vlan_dev))) {
if (!vlan_dev)
continue;
do_work(vlan_dev);
}
In that case we're sure that vlan_dev at least was used as a vlan, and won't
go away while we're holding rcu_read_lock().
However, if we don't hold rtnl_lock(), we can't be sure that that vlan_dev
is still in dev's vlan_list.
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
include/linux/if_vlan.h | 8 ++++++++
net/8021q/vlan.h | 6 ++++--
net/8021q/vlan_core.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 715c343..1cfc201 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -86,6 +86,8 @@ static inline int is_vlan_dev(struct net_device *dev)
extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
__be16 vlan_proto, u16 vlan_id);
+extern struct net_device *__vlan_find_dev_next(struct net_device *dev,
+ struct net_device *vlan_dev);
extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
extern u16 vlan_dev_vlan_id(const struct net_device *dev);
@@ -109,6 +111,12 @@ __vlan_find_dev_deep(struct net_device *real_dev,
return NULL;
}
+static struct net_device *__vlan_find_dev_next(struct net_device *dev,
+ struct net_device *vlan_dev)
+{
+ return NULL;
+}
+
static inline struct net_device *vlan_dev_real_dev(const struct net_device *dev)
{
BUG();
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index ba5983f..e13aeac 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -168,10 +168,12 @@ static inline struct net_device *vlan_find_dev(struct net_device *real_dev,
return NULL;
}
-#define vlan_group_for_each_dev(grp, i, dev) \
- for ((i) = 0; i < VLAN_PROTO_NUM * VLAN_N_VID; i++) \
+#define vlan_group_for_each_dev_from(grp, i, dev, from) \
+ for ((i) = from; i < VLAN_PROTO_NUM * VLAN_N_VID; i++) \
if (((dev) = __vlan_group_get_device((grp), (i) / VLAN_N_VID, \
(i) % VLAN_N_VID)))
+#define vlan_group_for_each_dev(grp, i, dev) \
+ vlan_group_for_each_dev_from(grp, i, dev, 0)
/* found in vlan_dev.c */
void vlan_dev_set_ingress_priority(const struct net_device *dev,
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 12e1606..20e1f92 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -89,6 +89,39 @@ struct net_device *__vlan_find_dev_deep(struct net_device *dev,
}
EXPORT_SYMBOL(__vlan_find_dev_deep);
+/* Must be called under rcu_read_lock(), returns next vlan_id used by a
+ * vlan device on dev, starting with vlan_id, or 0 if no vlan_id found.
+ */
+struct net_device *__vlan_find_dev_next(struct net_device *dev,
+ struct net_device *vlan_dev)
+{
+ struct net_device *real_dev = dev, *master_dev, *ret;
+ struct vlan_info *vlan_info;
+ struct vlan_group *grp;
+ u16 vlan_id = 0, i;
+
+ if (vlan_dev)
+ vlan_id = vlan_dev_priv(vlan_dev)->vlan_id + 1;
+
+ master_dev = netdev_master_upper_dev_get_rcu(real_dev);
+
+ if (master_dev)
+ real_dev = master_dev;
+
+ vlan_info = rcu_dereference(real_dev->vlan_info);
+
+ if (!vlan_info)
+ return NULL;
+
+ grp = &vlan_info->grp;
+
+ vlan_group_for_each_dev_from(grp, i, ret, vlan_id)
+ return ret;
+
+ return NULL;
+}
+EXPORT_SYMBOL(__vlan_find_dev_next);
+
struct net_device *vlan_dev_real_dev(const struct net_device *dev)
{
return vlan_dev_priv(dev)->real_dev;
--
1.7.1
^ permalink raw reply related
* [PATCH net-next v1 0/6] bonding: remove bond->vlan_list
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Andy Gospodarek, Patrick McHardy, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico
RFC -> v1: Got some feedback from Nikolay Aleksandrov (privately), tried to
address it, also fixed some bugs that I've found on the way. I
think it's ready to be considered a patchset for
review/inclusion in net-next.
The aim of this patchset is to remove bond->vlan_list completely, and use
8021q's standard functions instead of it.
The patchset is on top of Nik's latest two patches:
[net-next,v2,1/2] bonding: change the bond's vlan syncing functions with
the standard ones
[net-next,v2,2/2] bonding: unwind on bond_add_vlan failure
First two patches add two helper functions to vlan code:
bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
Here we change vlan_uses_dev() to be able to work under both rtnl
and rcu, and use it under rcu_read_lock() in bond_vlan_used().
vlan: add __vlan_find_dev_next()
This function takes dev and vlan_dev and returns the next vlan dev
that uses this dev. It can be used to cycle through the vlan list,
and not only by bonding - but by any network driver that uses its
private vlan list.
Next four patches actually convert bonding to use the new
functions/approach and remove the vlan_list completely.
This patchset solves several issues with bonding, simplify it overall,
RCUify further and add infrastructure to anyone else who'd like to use
8021q standard functions instead of their own vlan_list, which is quite
common amongst network drivers currently.
I'm testing it continuously currently, no issues found, will update on
anything.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_alb.c | 48 ++++++++----
drivers/net/bonding/bond_alb.h | 2 +-
drivers/net/bonding/bond_main.c | 163 ++++++---------------------------------
drivers/net/bonding/bonding.h | 16 ++--
include/linux/if_vlan.h | 8 ++
net/8021q/vlan.h | 6 +-
net/8021q/vlan_core.c | 36 ++++++++-
7 files changed, 115 insertions(+), 164 deletions(-)
^ permalink raw reply
* Re: Preferred method for configuration
From: Ben Hutchings @ 2013-08-08 9:44 UTC (permalink / raw)
To: Thomas Martitz; +Cc: Joe Perches, netdev@vger.kernel.org
In-Reply-To: <520332F4.5020206@hhi.fraunhofer.de>
On Thu, 2013-08-08 at 07:56 +0200, Thomas Martitz wrote:
> Resent with list in CC.
>
> Am 08.08.2013 07:41, schrieb Joe Perches:
> > On Thu, 2013-08-08 at 07:32 +0200, Thomas Martitz wrote:
> >> Hello,
> >>
> >> I'm developing a NIC driver and currently export some custom
> >> configuration settings and actions (e.g. enabling a loopback mode) via
> >> sysfs.
> >
> > Is there some reason you don't want to use ethtool?
> >
> >
>
> It doesn't allow for custom settings does it? For standards stuff I'm
> sure going to use ethtool.
You can always propose extensions to ethtool.
Note that the ethtool 'features' API already allows setting loopback and
there is a 'private flags' API through which you can expose up to 32
arbitrary named flags if they're really not at all generic.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: IGMP Unsolicited report interval patches
From: Hannes Frederic Sowa @ 2013-08-08 9:01 UTC (permalink / raw)
To: William Manley, netdev, bcrl, luky-37, sergei.shtylyov,
bhutchings, davem
In-Reply-To: <20130807010310.GH16410@order.stressinduktion.org>
On Wed, Aug 07, 2013 at 03:03:10AM +0200, Hannes Frederic Sowa wrote:
> On Tue, Aug 06, 2013 at 07:03:12PM +0100, William Manley wrote:
> > 4th version of the patches.
> >
> > The significant changes since last review are:
> >
> > 1. there is a new patch (2/3) as requested by Hannes.
>
> Thanks!
>
> > 2. the third patch now uses IN_DEV_CONF_GET in place of
> > IPV4_DEVCONF_ALL. This means that the unsolicited report interval can
> > now be configured on an interface-by-interface basis as I'd originally
> > intended but messed up in the implementation. One concern I have now
> > is that with this latest patch-set is that while
> > /proc/sys/net/ipv4/conf/eth0/igmp... will now have an effect
> > /proc/sys/net/ipv4/conf/all/igmp... will not. I'm not sure how to
> > resolve this.
>
> Hm, it seems to be come more difficult dealing with ranges.
>
> One way would be, to check the state bit for the devinet entry and chose the
> all value always but when the state bit for the interface for this entry is
> set. I'll have a look on how to do this.
We copy ipv4_devconf_dftl over to the per-interface ipv4_devconf as long no
address is assigned to the interface.
The copy-over is guarded by by the cnf.state bitmap. It is set to all ones in
ipv4_devconf_setall which gets called at the relevant places. Instead of
setting all bits we could just apply a bitmask which would still allow a
propagation of the default/*igmp* parameters.
The all namespace has special semantics and decision what to apply are done
per knob. I wouldn't fiddle with that.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH v2 net-next] net: Add low-latency/polling support for UDP multicast
From: Eliezer Tamir @ 2013-08-08 8:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Shawn Bohrer, davem, netdev, Amir Vadai, tomk
In-Reply-To: <1375906946.4004.45.camel@edumazet-glaptop>
On 07/08/2013 23:22, Eric Dumazet wrote:
> On Tue, 2013-08-06 at 14:51 -0500, Shawn Bohrer wrote:
>> Set the napi id for each socket in the multicast path to enable
>> low-latency/polling support.
>>
>> Signed-off-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
>> ---
>> v2 include ipv6 support
>
> This might help your workload, but I doubt it is generic enough.
>
> One UDP socket is supposed to receive traffic from many endpoints,
> so we have no guarantee all the received traffic will end on a single RX
> queue on the NIC.
>
> That's the same logic than RFS here.
>
> sk_mark_napi_id() in UDP are wrong IMHO.
>
> It should be guarded by the following test in
> __udp_queue_rcv_skb()
>
> if (inet_sk(sk)->inet_daddr) {
> sock_rps_save_rxhash(sk, skb);
> sk_mark_napi_id(sk, skb);
> }
>
> (To occur only for connected UDP sockets, where we are 100% sure all
> packets will use this same rxhash/rx queue)
This would also be safe if there is only one NIC and said NIC was
programmed to always place this socket's data on the same queue.
I don't have a good suggestion on how to detect this.
^ permalink raw reply
* Re: [PATCH] atm : fix /sys/devices/virtual/atm/X/carrier(ATM_PHY_SIG_UNKNOWN)
From: Philippe De Muyter @ 2013-08-08 8:34 UTC (permalink / raw)
To: chas3; +Cc: netdev
In-Reply-To: <201002181422.o1IEMrGo004602@thirdoffive.cmf.nrl.navy.mil>
On Thu, Feb 18, 2010 at 09:22:53AM -0500, Chas Williams (CONTRACTOR) wrote:
> In message <20100214175136.GA15891@frolo.macqel>,Philippe De Muyter writes:
> >cxacru itself does the right thing : as soon as carrier state is known,
> >signal is set to ATM_PHY_SIG_LOST or ATM_PHY_SIG_FOUND, but
> >atm_sysfs.c::show_carrier is wrong.
>
> so, i was thinking about something like this. atm_dev->signal is
> initializated to _LOST (because it is at this point), but most drivers
> will set ->signal to _FOUND since they dont handle detecting carrier
> when they finish their hardware initialization. the usb atm layer
> is the major exception here, and it appears that they already handle
> the atm_dev->signal status correctly. the minor exception being
> "unknown line state" -- i am going to assume LOST is the best choice
> for this case.
>
> please try this and let me know if it fixes your problem.
I had replied on Sun, 21 Feb 2010 15:42:04 +0100
Acked-by: Philippe De Muyter <phdm@macqel.be>
Did you push that patch, or a revised version of it, to Linus ?
Best regards
Philippe
>
> diff --git a/drivers/atm/adummy.c b/drivers/atm/adummy.c
> index 5effec6..2b9315f 100644
> --- a/drivers/atm/adummy.c
> +++ b/drivers/atm/adummy.c
> @@ -134,6 +134,7 @@ static int __init adummy_init(void)
> }
>
> list_add(&adummy_dev->entry, &adummy_devs);
> + atm_dev->signal = ATM_PHY_SIG_FOUND;
> out:
> return err;
>
> diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c
> index 8af2341..31e9baa 100644
> --- a/drivers/atm/ambassador.c
> +++ b/drivers/atm/ambassador.c
> @@ -2269,6 +2269,7 @@ static int __devinit amb_probe(struct pci_dev *pci_dev, const struct pci_device_
> // enable host interrupts
> interrupts_on (dev);
>
> + dev->atm_dev->signal = ATM_PHY_SIG_FOUND;
> out:
> return err;
>
> diff --git a/drivers/atm/atmtcp.c b/drivers/atm/atmtcp.c
> index 02ad83d..82ae3cc 100644
> --- a/drivers/atm/atmtcp.c
> +++ b/drivers/atm/atmtcp.c
> @@ -375,6 +375,7 @@ static int atmtcp_create(int itf,int persist,struct atm_dev **result)
> dev->dev_data = dev_data;
> PRIV(dev)->vcc = NULL;
> PRIV(dev)->persist = persist;
> + dev->signal = ATM_PHY_SIG_FOUND;
> if (result) *result = dev;
> return 0;
> }
> diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
> index cd5049a..b6cb376 100644
> --- a/drivers/atm/firestream.c
> +++ b/drivers/atm/firestream.c
> @@ -1923,6 +1923,7 @@ static int __devinit firestream_init_one (struct pci_dev *pci_dev,
>
> fs_dev->next = fs_boards;
> fs_boards = fs_dev;
> + atm_dev->signal = ATM_PHY_SIG_FOUND;
> return 0;
>
> err_out_free_atm_dev:
> diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
> index f7d6eba..ad2d919 100644
> --- a/drivers/atm/fore200e.c
> +++ b/drivers/atm/fore200e.c
> @@ -2637,6 +2637,7 @@ fore200e_init(struct fore200e* fore200e)
>
> /* all done, board initialization is now complete */
> fore200e->state = FORE200E_STATE_COMPLETE;
> + fore200e->atm_dev->signal = ATM_PHY_SIG_FOUND;
> return 0;
> }
>
> diff --git a/drivers/atm/he.c b/drivers/atm/he.c
> index e8c6529..ae83f9e 100644
> --- a/drivers/atm/he.c
> +++ b/drivers/atm/he.c
> @@ -1528,6 +1528,8 @@ he_start(struct atm_dev *dev)
> suni_init(he_dev->atm_dev);
> if (he_dev->atm_dev->phy && he_dev->atm_dev->phy->start)
> he_dev->atm_dev->phy->start(he_dev->atm_dev);
> +#else
> + he_dev->atm_dev->signal = ATM_PHY_SIG_FOUND;
> #endif /* CONFIG_ATM_HE_USE_SUNI */
>
> if (sdh) {
> diff --git a/drivers/atm/horizon.c b/drivers/atm/horizon.c
> index 4e49021..269c01f 100644
> --- a/drivers/atm/horizon.c
> +++ b/drivers/atm/horizon.c
> @@ -2827,6 +2827,7 @@ static int __devinit hrz_probe(struct pci_dev *pci_dev, const struct pci_device_
> dev->housekeeping.data = (unsigned long) dev;
> mod_timer(&dev->housekeeping, jiffies);
>
> + dev->atm_dev->signal = ATM_PHY_SIG_FOUND;
> out:
> return err;
>
> diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
> index 01f36c0..adc5eee 100644
> --- a/drivers/atm/idt77252.c
> +++ b/drivers/atm/idt77252.c
> @@ -3715,6 +3715,8 @@ idt77252_init_one(struct pci_dev *pcidev, const struct pci_device_id *id)
> err = -EIO;
> goto err_out_deinit_card;
> }
> +#else
> + dev->signal = ATM_PHY_SIG_FOUND;
> #endif /* CONFIG_ATM_IDT77252_USE_SUNI */
>
> card->sramsize = probe_sram(card);
> diff --git a/drivers/atm/lanai.c b/drivers/atm/lanai.c
> index 7fe7c32..446cc67 100644
> --- a/drivers/atm/lanai.c
> +++ b/drivers/atm/lanai.c
> @@ -2250,6 +2250,7 @@ static int __devinit lanai_dev_open(struct atm_dev *atmdev)
> "board_rev=%d\n", lanai->number,
> lanai->type==lanai2 ? "2" : "HB", (unsigned int) lanai->serialno,
> (unsigned int) lanai->serialno, lanai->board_rev);
> + atmdev->signal = ATM_PHY_SIG_FOUND;
> return 0;
>
> error_vcctable:
> diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
> index 5083840..02fb6a6 100644
> --- a/drivers/atm/nicstar.c
> +++ b/drivers/atm/nicstar.c
> @@ -815,14 +815,18 @@ static int __devinit ns_init_card(int i, struct pci_dev *pcidev)
> card->atmdev->link_rate = card->max_pcr;
> card->atmdev->phy = NULL;
>
> -#ifdef CONFIG_ATM_NICSTAR_USE_SUNI
> if (card->max_pcr == ATM_OC3_PCR)
> +#ifdef CONFIG_ATM_NICSTAR_USE_SUNI
> suni_init(card->atmdev);
> +#else
> + card->atmdev->signal = ATM_PHY_SIG_FOUND;
> #endif /* CONFIG_ATM_NICSTAR_USE_SUNI */
>
> -#ifdef CONFIG_ATM_NICSTAR_USE_IDT77105
> if (card->max_pcr == ATM_25_PCR)
> +#ifdef CONFIG_ATM_NICSTAR_USE_IDT77105
> idt77105_init(card->atmdev);
> +#else
> + card->atmdev->signal = ATM_PHY_SIG_FOUND;
> #endif /* CONFIG_ATM_NICSTAR_USE_IDT77105 */
>
> if (card->atmdev->phy && card->atmdev->phy->start)
> diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
> index 51eed67..0e505f5 100644
> --- a/drivers/atm/solos-pci.c
> +++ b/drivers/atm/solos-pci.c
> @@ -1245,7 +1245,7 @@ static int atm_init(struct solos_card *card)
> card->atmdev[i]->ci_range.vci_bits = 16;
> card->atmdev[i]->dev_data = card;
> card->atmdev[i]->phy_data = (void *)(unsigned long)i;
> - card->atmdev[i]->signal = ATM_PHY_SIG_UNKNOWN;
> + card->atmdev[i]->signal = ATM_PHY_SIG_LOST;
>
> skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
> if (!skb) {
> diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
> index 2e9635b..99712f0 100644
> --- a/drivers/atm/zatm.c
> +++ b/drivers/atm/zatm.c
> @@ -1345,6 +1345,8 @@ static int __devinit zatm_start(struct atm_dev *dev)
> zout(0xffffffff,IMR); /* enable interrupts */
> /* enable TX & RX */
> zout(zin(GMR) | uPD98401_GMR_SE | uPD98401_GMR_RE,GMR);
> +
> + dev->signal = ATM_PHY_SIG_FOUND;
> done:
> return error;
>
> diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
> index 56802d2..febe1f0 100644
> --- a/drivers/usb/atm/cxacru.c
> +++ b/drivers/usb/atm/cxacru.c
> @@ -803,7 +803,7 @@ static void cxacru_poll_status(struct work_struct *work)
> break;
>
> default:
> - atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
> + atm_dev->signal = ATM_PHY_SIG_LOST;
> atm_info(usbatm, "Unknown line state %02x\n", instance->line_status);
> break;
> }
> diff --git a/drivers/usb/atm/speedtch.c b/drivers/usb/atm/speedtch.c
> index 3e86240..fe19fe4 100644
> --- a/drivers/usb/atm/speedtch.c
> +++ b/drivers/usb/atm/speedtch.c
> @@ -536,7 +536,7 @@ static void speedtch_check_status(struct work_struct *work)
> break;
>
> case 0x08:
> - atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
> + atm_dev->signal = ATM_PHY_SIG_LOST;
> atm_info(usbatm, "ADSL line is blocked?\n");
> break;
>
> @@ -565,7 +565,7 @@ static void speedtch_check_status(struct work_struct *work)
> break;
>
> default:
> - atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
> + atm_dev->signal = ATM_PHY_SIG_LOST;
> atm_info(usbatm, "unknown line state %02x\n", status);
> break;
> }
> diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
> index fbea856..10899ec 100644
> --- a/drivers/usb/atm/usbatm.c
> +++ b/drivers/usb/atm/usbatm.c
> @@ -963,7 +963,7 @@ static int usbatm_atm_init(struct usbatm_data *instance)
>
> atm_dev->ci_range.vpi_bits = ATM_CI_MAX;
> atm_dev->ci_range.vci_bits = ATM_CI_MAX;
> - atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
> + atm_dev->signal = ATM_PHY_SIG_LOST;
>
> /* temp init ATM device, set to 128kbit */
> atm_dev->link_rate = 128 * 1000 / 424;
> diff --git a/include/linux/atmdev.h b/include/linux/atmdev.h
> index 817b237..6e3db35 100644
> --- a/include/linux/atmdev.h
> +++ b/include/linux/atmdev.h
> @@ -290,7 +290,6 @@ enum {
>
>
> #define ATM_PHY_SIG_LOST 0 /* no carrier/light */
> -#define ATM_PHY_SIG_UNKNOWN 1 /* carrier/light status is unknown */
> #define ATM_PHY_SIG_FOUND 2 /* carrier/light okay */
>
> #define ATM_ATMOPT_CLP 1 /* set CLP bit */
> diff --git a/net/atm/atm_sysfs.c b/net/atm/atm_sysfs.c
> index f693b78..a560857 100644
> --- a/net/atm/atm_sysfs.c
> +++ b/net/atm/atm_sysfs.c
> @@ -65,7 +65,7 @@ static ssize_t show_carrier(struct device *cdev,
> struct atm_dev *adev = to_atm_dev(cdev);
>
> pos += sprintf(pos, "%d\n",
> - adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
> + adev->signal == ATM_PHY_SIG_FOUND);
>
> return pos - buf;
> }
> diff --git a/net/atm/resources.c b/net/atm/resources.c
> index 9008290..2d5f95f 100644
> --- a/net/atm/resources.c
> +++ b/net/atm/resources.c
> @@ -38,7 +38,7 @@ static struct atm_dev *__alloc_atm_dev(const char *type)
> if (!dev)
> return NULL;
> dev->type = type;
> - dev->signal = ATM_PHY_SIG_UNKNOWN;
> + dev->signal = ATM_PHY_SIG_LOST;
> dev->link_rate = ATM_OC3_PCR;
> spin_lock_init(&dev->lock);
> INIT_LIST_HEAD(&dev->local);
^ permalink raw reply
* Re: macvtap bug: using smp_processor_id() in preemptible code
From: Thomas Huth @ 2013-08-08 8:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Vlad Yasevich, David S. Miller, Eric Dumazet
In-Reply-To: <1375889211.4004.28.camel@edumazet-glaptop>
Hi,
Am Wed, 07 Aug 2013 08:26:51 -0700
schrieb Eric Dumazet <eric.dumazet@gmail.com>:
> On Wed, 2013-08-07 at 16:43 +0200, Thomas Huth wrote:
> >
> > I am using macvtap (via KVM/virsh) on a s390 box. With the latest
> > kernel source from linux-next master branch, I suddenly get the
> > following error messages in the dmesg output:
> >
> > BUG: using smp_processor_id() in preemptible [00000000] code: vhost-45891/45892
> > caller is macvtap_do_read+0x45c/0x600 [macvtap]
> > CPU: 1 PID: 45892 Comm: vhost-45891 Not tainted 3.11.0-bisecttest #13
> > 000000010cab3a00 000000010cab3a10 0000000000000002 0000000000000000
> > 000000010cab3aa0 000000010cab3a18 000000010cab3a18 00000000001127b4
> > 0000000000000000 0000000000000001 0000000000000000 000000010000000b
> > 0000000000000060 000003fe00000008 0000000000000000 000000010cab3a70
> > 00000000006ea2f0 00000000001127b4 000000010cab3a00 000000010cab3a50
> > Call Trace:
> > ([<00000000001126ee>] show_trace+0x126/0x144)
> > [<00000000001127d2>] show_stack+0xc6/0xd4
> > [<000000000068bcec>] dump_stack+0x74/0xd8
> > [<0000000000481066>] debug_smp_processor_id+0xf6/0x114
> > [<000003ff802e9a18>] macvtap_do_read+0x45c/0x600 [macvtap]
> > [<000003ff802e9c1c>] macvtap_recvmsg+0x60/0x88 [macvtap]
> > [<000003ff80318c5e>] handle_rx+0x5b2/0x800 [vhost_net]
> > [<000003ff8028f77c>] vhost_worker+0x15c/0x1c4 [vhost]
> > [<000000000015f3ac>] kthread+0xd8/0xe4
> > [<00000000006934a6>] kernel_thread_starter+0x6/0xc
> > [<00000000006934a0>] kernel_thread_starter+0x0/0xc
> > 2 locks held by vhost-45891/45892:
> > #0: (&vq->mutex){+.+...}, at: [<000003ff80318718>] handle_rx+0x6c/0x800 [vhost_net]
> > #1: (rcu_read_lock){.+.+..}, at: [<000003ff802e98fe>] macvtap_do_read+0x342/0x600 [macvtap]
> >
> > Apart from these "annoying" error messages (they are repeated
> > continually while the KVM guest is running / macvtap is in use),
> > everything still seems to work fine, though.
> >
> > Since everything was still working fine in v3.10, I did some bisecting
> > and it seems like this commit introduced this problem:
> >
> > commit: ac4e4af1e59e16a018527ffa58d9d3f30bb96ca9
> > Subject: macvtap: Consistently use rcu functions
> >
> > I am not sure how to proceed here, this is my first bug report in this
> > area, so advice is welcome...
>
> Please try following fix :
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index a98fb0e..1c7aab4 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -912,8 +912,11 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
> done:
> rcu_read_lock();
> vlan = rcu_dereference(q->vlan);
> - if (vlan)
> + if (vlan) {
> + preempt_disable();
> macvlan_count_rx(vlan, copied - vnet_hdr_len, ret == 0, 0);
> + preempt_enable();
> + }
> rcu_read_unlock();
>
> return ret ? ret : copied;
Thank you very much for your fast reply and the fix, it indeed fixes
the messages about macvtap_do_read!
However, I now noticed that there are more messages, which I just did
not see before because my dmesg output was already flooded with the
messages about macvtap_do_read. The other messages are all about
macvlan_start_xmit:
BUG: using smp_processor_id() in preemptible [00000000] code: vhost-45897/45898
caller is macvlan_start_xmit+0x10a/0x1b4 [macvlan]
CPU: 1 PID: 45898 Comm: vhost-45897 Not tainted 3.11.0-bisecttest #16
00000001189b3960 00000001189b3970 0000000000000002 0000000000000000
00000001189b3a00 00000001189b3978 00000001189b3978 00000000001127b4
0000000000000000 0000000000000001 0000000000000000 000000000000000b
0000000000000060 000003fe00000008 0000000000000000 00000001189b39d0
00000000006ea2f0 00000000001127b4 00000001189b3960 00000001189b39b0
Call Trace:
([<00000000001126ee>] show_trace+0x126/0x144)
[<00000000001127d2>] show_stack+0xc6/0xd4
[<000000000068bdb8>] dump_stack+0x74/0xd4
[<0000000000481132>] debug_smp_processor_id+0xf6/0x114
[<000003ff802b72ca>] macvlan_start_xmit+0x10a/0x1b4 [macvlan]
[<000003ff802ea69a>] macvtap_get_user+0x982/0xbc4 [macvtap]
[<000003ff802ea92a>] macvtap_sendmsg+0x4e/0x60 [macvtap]
[<000003ff8031947c>] handle_tx+0x494/0x5ec [vhost_net]
[<000003ff8028f77c>] vhost_worker+0x15c/0x1c4 [vhost]
[<000000000015f3ac>] kthread+0xd8/0xe4
[<000000000069356e>] kernel_thread_starter+0x6/0xc
[<0000000000693568>] kernel_thread_starter+0x0/0xc
2 locks held by vhost-45897/45898:
#0: (&vq->mutex){+.+.+.}, at: [<000003ff8031903c>] handle_tx+0x54/0x5ec [vhost_net]
#1: (rcu_read_lock){.+.+..}, at: [<000003ff802ea53c>] macvtap_get_user+0x824/0xbc4 [macvtap]
Do you also have got an idea how to silence these messages?
Thomas
^ permalink raw reply
* Re: [PATCH 3/5] netfilter: add SYNPROXY core/target
From: Patrick McHardy @ 2013-08-08 8:24 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: pablo, netfilter-devel, netdev, mph, as
In-Reply-To: <20130808100448.010cd96d@redhat.com>
On Thu, Aug 08, 2013 at 10:04:48AM +0200, Jesper Dangaard Brouer wrote:
>
> On Wed, 7 Aug 2013 22:56:03 +0200 Patrick McHardy <kaber@trash.net> wrote:
> > On Wed, Aug 07, 2013 at 10:26:00PM +0200, Jesper Dangaard Brouer wrote:
> > > On Wed, 7 Aug 2013 19:42:49 +0200 Patrick McHardy <kaber@trash.net> wrote:
>
> [...]
> > > Besides shouldn't nth->ack_seq be zero, in a SYN packet? This is the
> > > SYN "replayed" towards the server right?
> > >
> > > I also pointed to this in an earlier patch Martin showed me, but he
> > > reported that changing this resulted in bad behavior. So, I would
> > > request Martin to re-test this part.
> >
> > Right, it should be zero, but it doesn't matter since the ACK flag isn't
> > set. This is used to propagate the sequence number to the hook function
> > to initialize the sequence adjustment data. While in the target function,
> > we don't have any connection tracking state to store this in. We could
> > set it to zero after that, but it shouldn't matter.
>
> I think it deserves a comment in the code, that you are using ack_seq,
> to relay this information to the hook, as its not obvious.
Agreed, I've added a comment.
> And I think we should set it to zero after that, else it will be
> visible on the wire, and wireshark complains (with a warning) when it
> sees pure SYN packets with a non-zero ACK number (Martin send me a dump
> some time ago, and I just checked).
I'm a bit reluctant to do the entire "make skb writable, change packet,
update checksum" dance for a cosmetic issue when wireshark should in
fact ignore the value since the ACK flag is not set. I'll give it a try
and see how ugly it gets.
> p.s. thanks for working on this module, which we discussed during the
> Netfilter Workshop 2013.
Well, I think its pretty cool considering the numbers ;)
^ permalink raw reply
* Re: [PATCH 3/5] netfilter: add SYNPROXY core/target
From: Jesper Dangaard Brouer @ 2013-08-08 8:04 UTC (permalink / raw)
To: Patrick McHardy; +Cc: pablo, netfilter-devel, netdev, mph, as
In-Reply-To: <20130807205602.GA21463@macbook.localnet>
On Wed, 7 Aug 2013 22:56:03 +0200 Patrick McHardy <kaber@trash.net> wrote:
> On Wed, Aug 07, 2013 at 10:26:00PM +0200, Jesper Dangaard Brouer wrote:
> > On Wed, 7 Aug 2013 19:42:49 +0200 Patrick McHardy <kaber@trash.net> wrote:
[...]
> > Besides shouldn't nth->ack_seq be zero, in a SYN packet? This is the
> > SYN "replayed" towards the server right?
> >
> > I also pointed to this in an earlier patch Martin showed me, but he
> > reported that changing this resulted in bad behavior. So, I would
> > request Martin to re-test this part.
>
> Right, it should be zero, but it doesn't matter since the ACK flag isn't
> set. This is used to propagate the sequence number to the hook function
> to initialize the sequence adjustment data. While in the target function,
> we don't have any connection tracking state to store this in. We could
> set it to zero after that, but it shouldn't matter.
I think it deserves a comment in the code, that you are using ack_seq,
to relay this information to the hook, as its not obvious.
And I think we should set it to zero after that, else it will be
visible on the wire, and wireshark complains (with a warning) when it
sees pure SYN packets with a non-zero ACK number (Martin send me a dump
some time ago, and I just checked).
p.s. thanks for working on this module, which we discussed during the
Netfilter Workshop 2013.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: skbs delivered to 'wrong' packet_type handler
From: Erik Hugne @ 2013-08-08 7:43 UTC (permalink / raw)
To: Ying Xue; +Cc: netdev, tipc-discussion
In-Reply-To: <520300A4.10909@windriver.com>
On Thu, Aug 08, 2013 at 10:21:24AM +0800, Ying Xue wrote:
> But I am sure the patch in below link can
> definitely fix your met issue although it's not a final version to be
> submitted net-next:
>
> http://permalink.gmane.org/gmane.network.tipc.general/4159
>
Yes it does, thanks!
//E
^ permalink raw reply
* Re: some questions of tcp congestion window
From: Dong Fang @ 2013-08-08 19:34 UTC (permalink / raw)
To: netdev, Dong Fang
In-Reply-To: <5203F0DD.1090007@gmail.com>
On 08/08/2013 03:26 PM, Dong Fang wrote:
> hi, all
>
> I'am reading the tcp/ip network source code(kernel 3.10), i was
> fogged by the congestion window. so, i want to confirm something
> about it:
>
> Note: sack is disable. and A is sender, B is receiver.
>
> 1. at time t, the number of packets in flight is 100. like this：
> u, u+1, u+2, ..., u+99
> and suppose u is lost and all other packets are not. so bwtween
> t:t+RTT we would have retransmitted u and received 99 dupack.
> all of this dupack's ack == u, right?
>
> when A recieved 3 dupack, it changed to TCP_CA_Recovery state.
> in this state, the congestion window won't grow any more.
> then, A retransmit U packet. after that, if A receive a new ack,
> acked all the packets in flight, so the sock state is changed
> to TCP_CA_Open. right?
>
> 2. at time t, the number of packets in flight is 100, like this:
> u, u+1, u+2, ..., u+99
> and suppose u and u+5 is lost and all other packets are not.
> between t:t+RTT, we should have retransmitted u and received 99
> dupack, all of this dupack's ack == u, right?
>
> when A recieved 3 dupack, it changed to Recovery state, then
> retransmit U packet, after that, if A receive a new ack,
> this ack is only acked for u+5, at this time, current sshresh =
> cwnd/2 + 5(the first 5 packets was acked), but cwnd > sshresh.
> so A won't send any packets to B, and B won't send any ack to
> A too, because B have beed send 99 dupack and 1 new ack, it
> have done its work, right? the only way to let B send ack to
> A is, the retransmit timeout of u+5 packet.
>
> when A was retransmit u+5 packet to B, then B send packet ack
> for u+100 to A, this time, the A's cwnd == sshresh << 1, enter
> CA progress, right?
can anybody help me? :)
^ permalink raw reply
* some questions of tcp congestion window
From: Dong Fang @ 2013-08-08 19:26 UTC (permalink / raw)
To: netdev
hi, all
I'am reading the tcp/ip network source code(kernel 3.10), i was
fogged by the congestion window. so, i want to confirm something
about it:
Note: sack is disable. and A is sender, B is receiver.
1. at time t, the number of packets in flight is 100. like this：
u, u+1, u+2, ..., u+99
and suppose u is lost and all other packets are not. so bwtween
t:t+RTT we would have retransmitted u and received 99 dupack.
all of this dupack's ack == u, right?
when A recieved 3 dupack, it changed to TCP_CA_Recovery state.
in this state, the congestion window won't grow any more.
then, A retransmit U packet. after that, if A receive a new ack,
acked all the packets in flight, so the sock state is changed
to TCP_CA_Open. right?
2. at time t, the number of packets in flight is 100, like this:
u, u+1, u+2, ..., u+99
and suppose u and u+5 is lost and all other packets are not.
between t:t+RTT, we should have retransmitted u and received 99
dupack, all of this dupack's ack == u, right?
when A recieved 3 dupack, it changed to Recovery state, then
retransmit U packet, after that, if A receive a new ack,
this ack is only acked for u+5, at this time, current sshresh =
cwnd/2 + 5(the first 5 packets was acked), but cwnd > sshresh.
so A won't send any packets to B, and B won't send any ack to
A too, because B have beed send 99 dupack and 1 new ack, it
have done its work, right? the only way to let B send ack to
A is, the retransmit timeout of u+5 packet.
when A was retransmit u+5 packet to B, then B send packet ack
for u+100 to A, this time, the A's cwnd == sshresh << 1, enter
CA progress, right?
^ permalink raw reply
* [PATCH] net/tg3: Support shutdown on PCI device
From: Gavin Shan @ 2013-08-08 7:26 UTC (permalink / raw)
To: netdev; +Cc: nsujir, davem, Gavin Shan
Before we tries to load new kernel with "kexec", we cound't bring
the tg3 NICs down manually because dhcp daemon keeps them up. So
there're still lots of traffic (DMA operations) on the tg3 card
from the old kernel while running the new kernel. It leads to
unnecessary EEH errors on Power platform because the network traffic
(from old kernel) breaks what we have in the IOMMU, which is built
by the new kernel.
ifconfig eth0 down
kexec -l vmlinux -c "root=/dev/sda5 debug" && kexec -f -e
The patch adds the shutdown interface for the PCI device to make
things more robust. The PCI device can be put to silent state before
loading the new kernel and to avoid unnecessary EEH errors.
Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
drivers/net/ethernet/broadcom/tg3.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index ddebc7a..dc6178b 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -17663,6 +17663,17 @@ static void tg3_remove_one(struct pci_dev *pdev)
}
}
+static void tg3_shutdown_one(struct pci_dev *pdev)
+{
+ struct net_device *dev = pci_get_drvdata(pdev);
+
+ if (!dev)
+ return;
+
+ netif_device_detach(dev);
+ tg3_remove_one(pdev);
+}
+
#ifdef CONFIG_PM_SLEEP
static int tg3_suspend(struct device *device)
{
@@ -17909,6 +17920,7 @@ static struct pci_driver tg3_driver = {
.id_table = tg3_pci_tbl,
.probe = tg3_init_one,
.remove = tg3_remove_one,
+ .shutdown = tg3_shutdown_one,
.err_handler = &tg3_err_handler,
.driver.pm = &tg3_pm_ops,
};
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 3/5] netfilter: add SYNPROXY core/target
From: Patrick McHardy @ 2013-08-08 6:34 UTC (permalink / raw)
To: Eric Dumazet; +Cc: pablo, netfilter-devel, netdev, mph, jesper.brouer, as
In-Reply-To: <20130807233707.GA23937@macbook.localnet>
On Thu, Aug 08, 2013 at 01:37:11AM +0200, Patrick McHardy wrote:
> On Wed, Aug 07, 2013 at 03:11:54PM -0700, Eric Dumazet wrote:
> > On Wed, 2013-08-07 at 19:42 +0200, Patrick McHardy wrote:
> >
> > Also please check your uses of kfree_skb() .
> >
> > Some of them would better be consume_skb() (for example in
> > ipv4_synproxy_hook())
>
> I'll look into that.
Agreed for this case. The only other two kfree_skb()s are actually
error paths.
^ permalink raw reply
* [PATCH] net: stmmac: Fixed the condition of extend_desc for jumbo frame
From: Byungho An @ 2013-08-08 6:30 UTC (permalink / raw)
To: netdev
Cc: 'Giuseppe CAVALLARO', '김국진',
davem, cpgs
This patch fixed the condition of extend_desc for jumbo frame.
There is no check routine for extend_desc in the stmmac_jumbo_frm function.
Even though extend_desc is set if dma_tx is used instead of dma_etx.
It causes kernel panic.
Signed-off-by: Byungho An <bh74.an@samsung.com>
---
drivers/net/ethernet/stmicro/stmmac/ring_mode.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
index c9d942a..1ef9d8a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
@@ -33,10 +33,15 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
struct stmmac_priv *priv = (struct stmmac_priv *)p;
unsigned int txsize = priv->dma_tx_size;
unsigned int entry = priv->cur_tx % txsize;
- struct dma_desc *desc = priv->dma_tx + entry;
+ struct dma_desc *desc;
unsigned int nopaged_len = skb_headlen(skb);
unsigned int bmax, len;
+ if (priv->extend_desc)
+ desc = (struct dma_desc *)(priv->dma_etx + entry);
+ else
+ desc = priv->dma_tx + entry;
+
if (priv->plat->enh_desc)
bmax = BUF_SIZE_8KiB;
else
@@ -54,7 +59,11 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
STMMAC_RING_MODE);
wmb();
entry = (++priv->cur_tx) % txsize;
- desc = priv->dma_tx + entry;
+
+ if (priv->extend_desc)
+ desc = (struct dma_desc *)(priv->dma_etx + entry);
+ else
+ desc = priv->dma_tx + entry;
desc->des2 = dma_map_single(priv->device, skb->data + bmax,
len, DMA_TO_DEVICE);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 3/5] netfilter: add SYNPROXY core/target
From: Patrick McHardy @ 2013-08-08 6:22 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: pablo, netfilter-devel, netdev, mph, as
In-Reply-To: <20130807205602.GA21463@macbook.localnet>
On Wed, Aug 07, 2013 at 10:56:03PM +0200, Patrick McHardy wrote:
> On Wed, Aug 07, 2013 at 10:26:00PM +0200, Jesper Dangaard Brouer wrote:
> > On Wed, 7 Aug 2013 19:42:49 +0200
> > Patrick McHardy <kaber@trash.net> wrote:
> >
> > Besides when using net->proc_net_stat, then the first entry is usually
> > "entries" which is not percpu, this will likely confusing the tool:
> > lnstat -f synproxy -c 42
>
> I'll look into that.
Ok right, the first field must contains something that is not per-CPU.
Unfortunately I don't have anything to put there and I really don't want
to keep any global state. The two possibilities I see are:
- a dummy field
- the number of proxied connections, but not using a global counter but
gathered by iterating over the entire conntrack hash.
Any opinions?
^ 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