* [PATCH] net/core: another bug fix related to dst_neigh_lookup/dst_neigh_lookup_skb
From: Zhouyi Zhou @ 2013-03-13 8:40 UTC (permalink / raw)
To: netdev; +Cc: stable, trivial
I do a
/home/zzy/linux-stable#find . -name "*.c"|xargs grep -A 2 dst_neigh_lookup
and find that there are two places in drivers/infiniband/hw/cxgb4/cm.c
do not treat the error return of dst_neigh_lookup/dst_neigh_lookup_skb
Signed-off-by: Zhouyi Zhou <zhouzhouyi <at> gmail.com>
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 565bfb1..b5d6c4a 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -1575,6 +1575,12 @@ static int c4iw_reconnect(struct c4iw_ep *ep)
neigh = dst_neigh_lookup(ep->dst,
&ep->com.cm_id->remote_addr.sin_addr.s_addr);
+ if (!neigh) {
+ pr_err("%s - cannot alloc l2e.\n", __func__);
+ err = -ENOMEM;
+ goto fail4;
+ }
+
/* get a l2t entry */
if (neigh->dev->flags & IFF_LOOPBACK) {
PDBG("%s LOOPBACK\n", __func__);
@@ -3052,7 +3058,11 @@ static int rx_pkt(struct c4iw_dev *dev, struct
sk_buff *skb)
}
dst = &rt->dst;
neigh = dst_neigh_lookup_skb(dst, skb);
-
+ if (!neigh){
+ pr_err("%s - failed to allocate l2t entry!\n",
+ __func__);
+ goto free_dst;
+ }
if (neigh->dev->flags & IFF_LOOPBACK) {
pdev = ip_dev_find(&init_net, iph->daddr);
e = cxgb4_l2t_get(dev->rdev.lldi.l2t, neigh,
Thanks for your review
^ permalink raw reply related
* Re: [PATCH v2 0/5] cpsw interrupt pacing and get/set phy setting implementation
From: David Miller @ 2013-03-13 8:39 UTC (permalink / raw)
To: mugunthanvnm; +Cc: netdev, devicetree-discuss, linux-omap, b-cousson, paul
In-Reply-To: <1363079798-18147-1-git-send-email-mugunthanvnm@ti.com>
From: Mugunthan V N <mugunthanvnm@ti.com>
Date: Tue, 12 Mar 2013 14:46:33 +0530
> This patch serires implements the following features in CPSW driver
> * get/set phy link settings
> * interrupt pacing
> * get phy id via ioctl cmd SIOCGMIIPHY
>
> Changes from initial version
> * Made active-slave common for cpts, ethtool and SIOCGMIIPHY ioctl
> * Cleaned CPSW DT binding documentation by seperating slave nodes
> under sub-section
> * implemented get phy id via ioctl cmd SIOCGMIIPHY
Series applied, thanks.
^ permalink raw reply
* Re: [ 000/100] 3.8.3-stable review
From: David Miller @ 2013-03-13 8:33 UTC (permalink / raw)
To: a.heider; +Cc: gregkh, linux-kernel, torvalds, akpm, stable, netdev
In-Reply-To: <CAHsu+b9O+JG=v6FgAEG4KOVDGScFiZYQm_nqXOQ9NSae3D3hwA@mail.gmail.com>
From: Andre Heider <a.heider@gmail.com>
Date: Wed, 13 Mar 2013 09:02:46 +0100
> On Tue, Mar 12, 2013 at 11:30 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> This is the start of the stable review cycle for the 3.8.3 release.
>> There are 100 patches in this series, all will be posted as a response
>> to this one. If anyone has any issues with these being applied, please
>> let me know.
>
> Hi,
>
> I'm missing a drivers/net/ethernet/atheros/atl1c patch, namely "atl1c:
> restore buffer state".
> In fact, it seems the whole netdev stable patch queue is missing:
> http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
>
> Unsure what the workflow is here, how are these supposed to make to stable?
I submit those patches at a time of my own choosing, and if you
read stable@vger.kernel.org you will see when I do that.
I let patches cook in Linus's tree for a week or so before submitting
to -stable so that any bugs in those changes are more likely to be
discovered before the change is in -stable too.
^ permalink raw reply
* Re: usbnet: use_mem_cache to avoid GFP_ATOMIC
From: Oliver Neukum @ 2013-03-13 8:14 UTC (permalink / raw)
To: Facix Booka; +Cc: netdev@vger.kernel.org
In-Reply-To: <1363114759.37226.YahooMailNeo@web126005.mail.ne1.yahoo.com>
On Tuesday 12 March 2013 11:59:19 Facix Booka wrote:
>I would like to hand over my work to you kernel people. May be my problems
>and fixes are only sience fiction to other problems in somewhere else what I
>did not see. Default value can be use_mem_cache=1.
>Print out "eth0: kevent 2 may have been dropped" may need attention.
Hi,
I am sorry, but this won't do.
1. It is too subtle
2. the resume() code path
3. use a pool if this is a real problem
Regards
Oliver
^ permalink raw reply
* Re: [ 000/100] 3.8.3-stable review
From: Andre Heider @ 2013-03-13 8:02 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, torvalds, akpm, stable, netdev, davem
In-Reply-To: <20130312223122.884099393@linuxfoundation.org>
On Tue, Mar 12, 2013 at 11:30 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> This is the start of the stable review cycle for the 3.8.3 release.
> There are 100 patches in this series, all will be posted as a response
> to this one. If anyone has any issues with these being applied, please
> let me know.
Hi,
I'm missing a drivers/net/ethernet/atheros/atl1c patch, namely "atl1c:
restore buffer state".
In fact, it seems the whole netdev stable patch queue is missing:
http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
Unsure what the workflow is here, how are these supposed to make to stable?
Thanks,
Andre
^ permalink raw reply
* Re: [PATCH] igb: SR-IOV init reordering
From: Jeff Kirsher @ 2013-03-13 7:51 UTC (permalink / raw)
To: Alex Williamson; +Cc: gregory.v.rose, netdev, ddutile, linux-kernel, sassmann
In-Reply-To: <20130312212533.19552.92159.stgit@bling.home>
[-- Attachment #1: Type: text/plain, Size: 610 bytes --]
On Tue, 2013-03-12 at 15:25 -0600, Alex Williamson wrote:
> igb is ineffective at setting a lower total VFs because:
>
> int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
> {
> ...
> /* Shouldn't change if VFs already enabled */
> if (dev->sriov->ctrl & PCI_SRIOV_CTRL_VFE)
> return -EBUSY;
>
> Swap init ordering.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I have added the patch to my igb queue, thanks!
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] igb: Fix null pointer dereference
From: Jeff Kirsher @ 2013-03-13 7:51 UTC (permalink / raw)
To: Alex Williamson; +Cc: gregory.v.rose, netdev, ddutile, linux-kernel, sassmann
In-Reply-To: <20130312200833.17121.47670.stgit@bling.home>
[-- Attachment #1: Type: text/plain, Size: 1056 bytes --]
On Tue, 2013-03-12 at 14:09 -0600, Alex Williamson wrote:
> The max_vfs= option has always been self limiting to the number of VFs
> supported by the device. fa44f2f1 added SR-IOV configuration via
> sysfs, but in the process broke this self correction factor. The
> failing path is:
>
> igb_probe
> igb_sw_init
> if (max_vfs > 7) {
> adapter->vfs_allocated_count = 7;
> ...
> igb_probe_vfs
> igb_enable_sriov(, max_vfs)
> if (num_vfs > 7) {
> err = -EPERM;
> ...
>
> This leaves vfs_allocated_count = 7 and vf_data = NULL, so we bomb out
> when igb_probe finally calls igb_reset. It seems like a really bad
> idea, and somewhat pointless, to set vfs_allocated_count separate from
> vf_data, but limiting max_vfs is enough to avoid the null pointer.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I have added the patch to my igb queue, thanks!
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] net/ieee802154/6lowpan: Fix initialization for fragment offset
From: Wolf-Bastian Pöttner @ 2013-03-13 7:50 UTC (permalink / raw)
To: Linux-zigbee-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
offset has to be initialized, otherwise the *first* fragment will
be discarded and reassembly cannot happen.
Reviewed-by: Alan Ott <alan-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org>
---
net/ieee802154/6lowpan.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index ed3debe..2db7261 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -745,7 +745,7 @@ lowpan_process_data(struct sk_buff *skb)
{
struct lowpan_fragment *frame;
/* slen stores the rightmost 8 bits of the 11 bits length */
- u8 slen, offset;
+ u8 slen, offset = 0;
u16 len, tag;
bool found = false;
--
1.7.5.4
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
^ permalink raw reply related
* Re: [RFC PATCH] qmi_wwan: set correct altsetting for Gobi 1K devices
From: Bjørn Mork @ 2013-03-13 7:13 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-usb, netdev
In-Reply-To: <2400f481-f2eb-440d-b74d-ff9bfccd1179@email.android.com>
Bjørn Mork <bjorn@mork.no> writes:
>
> Risking that I am trying to be too smart again. .. But I really think
> we can fix this in a more generic way by making the probe behave more
> like it did before. How about just letting it continue without erring
> out until it has called usbnet_get_endpoints? That should work without
> needing any quirks.
I was thinking about something like this patch, dropping both the
unnecessary verification of bNumEndpoints and of the CDC functional
descriptors. If we find a CDC Union, then fine - we use it to locate
the data interface. If we find a CDC Ether, then fine - we use it to set
the MAC address.
This should end up calling usbnet_get_endpoints (first thing
qmi_wwan_register_subdriver does), and succeed if any data interface
altsetting have the necessary endpoints.
There really are few reasons to do any strict verification step in the
probe in this driver, because there really are no usable markers on the
functions. That's why the driver does explicit interface matching in
the first place. So it makes sense to allow the probe to succeed if
collecting the 3 required endpoints is at all possible.
It will of course succeed on a number of unsupported interfaces if
dynamical IDs are used. But with nothing to differentiate a QMI/wwan
function from other functions, there is really nothing we can do to
prevent that. If we are to support dynamic IDs, then there will be false
positives. The user can manually unbind the driver from unwanted
matches in this case.
Only build tested for now. I will test on the devices I have before
submitting, if this works for you:
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index efb5c7c..1cfa80c 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -139,18 +139,11 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));
- /* control and data is shared? */
- if (intf->cur_altsetting->desc.bNumEndpoints == 3) {
- info->control = intf;
- info->data = intf;
- goto shared;
- }
-
- /* else require a single interrupt status endpoint on control intf */
- if (intf->cur_altsetting->desc.bNumEndpoints != 1)
- goto err;
+ /* set up initial state */
+ info->control = intf;
+ info->data = intf;
- /* and a number of CDC descriptors */
+ /* some vendors add CDC descriptors */
while (len > 3) {
struct usb_descriptor_header *h = (void *)buf;
@@ -207,25 +200,17 @@ next_desc:
buf += h->bLength;
}
- /* did we find all the required ones? */
- if (!(found & (1 << USB_CDC_HEADER_TYPE)) ||
- !(found & (1 << USB_CDC_UNION_TYPE))) {
- dev_err(&intf->dev, "CDC functional descriptors missing\n");
- goto err;
- }
-
- /* verify CDC Union */
- if (desc->bInterfaceNumber != cdc_union->bMasterInterface0) {
- dev_err(&intf->dev, "bogus CDC Union: master=%u\n", cdc_union->bMasterInterface0);
- goto err;
- }
-
- /* need to save these for unbind */
- info->control = intf;
- info->data = usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0);
- if (!info->data) {
- dev_err(&intf->dev, "bogus CDC Union: slave=%u\n", cdc_union->bSlaveInterface0);
- goto err;
+ /* using two interfaces if we found a CDC Union */
+ if (cdc_union) {
+ if (desc->bInterfaceNumber != cdc_union->bMasterInterface0) {
+ dev_err(&intf->dev, "bogus CDC Union: master=%u\n", cdc_union->bMasterInterface0);
+ goto err;
+ }
+ info->data = usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0);
+ if (!info->data) {
+ dev_err(&intf->dev, "bogus CDC Union: slave=%u\n", cdc_union->bSlaveInterface0);
+ goto err;
+ }
}
/* errors aren't fatal - we can live with the dynamic address */
@@ -234,12 +219,13 @@ next_desc:
usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress);
}
- /* claim data interface and set it up */
- status = usb_driver_claim_interface(driver, info->data, dev);
- if (status < 0)
- goto err;
+ /* claim separate data interface? */
+ if (info->control != info->data) {
+ status = usb_driver_claim_interface(driver, info->data, dev);
+ if (status < 0)
+ goto err;
+ }
-shared:
status = qmi_wwan_register_subdriver(dev);
if (status < 0 && info->control != info->data) {
usb_set_intfdata(info->data, NULL);
^ permalink raw reply related
* Re: [E1000-devel] [patch] ixgbe: in shutdown, do netif_running() under rtnl_lock
From: Jeff Kirsher @ 2013-03-13 6:33 UTC (permalink / raw)
To: akepner; +Cc: netdev, e1000-devel
In-Reply-To: <20130308220510.GJ21267@riverbed.com>
[-- Attachment #1: Type: text/plain, Size: 3295 bytes --]
On Fri, 2013-03-08 at 14:05 -0800, akepner wrote:
>
> During shutdown it's possible for __dev_close() (which holds
> rtnl_lock) to clear the __LINK_STATE_START bit, and for ixgbe
> to then read that bit (without holding rtnl_lock), and then
> not fail to free irqs, etc. The result is a crash like this:
>
> ------------[ cut here ]------------
> kernel BUG at drivers/pci/msi.c:313!
> invalid opcode: 0000 [#1] SMP
> last sysfs
> file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map
> CPU 1
> Pid: 5910, comm: reboot Tainted: P ---------------- 2.6.32
> #1 empty
> RIP: 0010:[<ffffffff81305c2b>] [<ffffffff81305c2b>] free_msi_irqs
> +0x11b/0x130
> RSP: 0018:ffff880185c9bc88 EFLAGS: 00010282
> RAX: ffff880219f58bc0 RBX: ffff88021ac53b00 RCX: 0000000000000000
> RDX: 0000000000000001 RSI: 0000000000000246 RDI: 000000000000004a
> RBP: ffff880185c9bcc8 R08: 0000000000000002 R09: 0000000000000106
> R10: 0000000000000000 R11: 0000000000000006 R12: ffff88021e524778
> R13: 0000000000000001 R14: ffff88021e524000 R15: 0000000000000000
> FS: 00007f90821b7700(0000) GS:ffff880028220000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 00007f90818bd010 CR3: 0000000132c64000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process reboot (pid: 5910, threadinfo ffff880185c9a000, task
> ffff88021bf04a80)
> Stack:
> ffff880185c9bc98 000000018130529d ffff880185c9bcc8 ffff88021e524000
> <0> 0000000000000004 ffff88021948c700 0000000000000000
> ffff880185c9bda7
> <0> ffff880185c9bce8 ffffffff81305cbd ffff880185c9bce8
> ffff88021948c700
> Call Trace:
> [<ffffffff81305cbd>] pci_disable_msix+0x3d/0x50
> [<ffffffffa00501d5>] ixgbe_reset_interrupt_capability+0x65/0x90
> [ixgbe]
> [<ffffffffa00512f6>] ixgbe_clear_interrupt_scheme+0xb6/0xd0 [ixgbe]
> [<ffffffffa005330b>] __ixgbe_shutdown+0x5b/0x200 [ixgbe]
> [<ffffffffa00534ca>] ixgbe_shutdown+0x1a/0x60 [ixgbe]
> [<ffffffff812f6c7c>] pci_device_shutdown+0x2c/0x50
> [<ffffffff813727fb>] device_shutdown+0x4b/0x160
> [<ffffffff8107d98c>] kernel_restart_prepare+0x2c/0x40
> ehci timer_action, mod_timer io_watchdog
> [<ffffffff8107d9e6>] kernel_restart+0x16/0x60
> [<ffffffff8107dbfd>] sys_reboot+0x1ad/0x200
> [<ffffffff811676cf>] ? __d_free+0x3f/0x60
> [<ffffffff81167748>] ? d_free+0x58/0x60
> [<ffffffff8116f7c0>] ? mntput_no_expire+0x30/0x100
> [<ffffffff81152b11>] ? __fput+0x191/0x200
> [<ffffffff816565fe>] ? do_page_fault+0x3e/0xa0
> [<ffffffff8100b132>] system_call_fastpath+0x16/0x1b
> Code: 4c 89 ef e8 98 8c e3 ff 4d 39 f4 48 8b 43 10 75 cf 48 83 c4 18
> 5b 41 5c
> 41 5d 41 5e 41 5f c9 c3 49 8b 7d 20 e8 07 5a d3 ff eb c9 <0f> 0b 0f 1f
> 00 eb fb
> 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00
> ehci timer_action, mod_timer io_watchdog
> RIP [<ffffffff81305c2b>] free_msi_irqs+0x11b/0x130
> RSP <ffff880185c9bc88>
> ---[ end trace 27de882a0fe75593 ]---
>
> (This was seen on a pretty old kernel/driver, but looks like
> the same bug is still possible.)
>
> Signed-off-by: <akepner@riverbed.com>
Thanks, I have added the patch to my ixgbe queue of patches.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 0/4] Allow bridge to function in non-promisc mode
From: "Oleg A. Arkhangelsky" @ 2013-03-13 6:22 UTC (permalink / raw)
To: Vlad Yasevich, netdev@vger.kernel.org; +Cc: bridge@lists.linux-foundation.org
In-Reply-To: <1363139126-13396-1-git-send-email-vyasevic@redhat.com>
13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
> The series adds an ability for the bridge to function in non-promiscuous mode.
What is the practical applications for such setup? In other words,
in which cases I would want to put bridge into non-promiscuous
mode and specify some uplink ports?
--
wbr, Oleg.
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
^ permalink raw reply
* r8169 and flapping links
From: Adam Jacob Muller @ 2013-03-13 6:05 UTC (permalink / raw)
To: netdev
Hi,
I have a pair of RTL8168B-based network cards with SFP's installed in
them (http://www.startech.com/~PEX1000SFP)
They have some extremely unusual behavior that I'm curious if anyone on
the list has ever seen or could shed some light on.
Sometimes after bringing the network card up the network driver will
continuously flap like:
r8169 0000:01:00.0 eth2: link down
r8169 0000:01:00.0 eth2: link up
r8169 0000:01:00.0 eth2: link down
r8169 0000:01:00.0 eth2: link up
The link is obviously not usable in that state.
On occasion though, some combination of down/up'ing the interface will
actually bring things into a stable state and everything works well.
Interestingly, the hardware link light does not fluctuate while this
happens.
I've tried with the kernel-based r8169 driver, and the r8168 driver from
the Realtek site, but both exhibit the same behavior.
I do see a number of "similar" reports online regarding this issue, has
anyone here seen anything like this before? Neither raising the module
debug level to 16 and raising msglvl with ethool produced nothing
interesting. Any suggestions on where to look for more debug (or a
better list to ask, if this isn't the right place) would be much
appreciated.
Thanks!
-Adam
^ permalink raw reply
* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Eric Dumazet @ 2013-03-13 6:13 UTC (permalink / raw)
To: Bill Fink; +Cc: Thomas Graf, Chris Friesen, Vimal, netdev, shemminger
In-Reply-To: <20130313020156.c9dd9841.billfink@mindspring.com>
On Wed, 2013-03-13 at 02:01 -0400, Bill Fink wrote:
> The last time this was discussed appears to be (on 2011-03-28):
>
> http://marc.info/?l=linux-netdev&m=130128741907282&w=2
>
> where Maciej Żenczykowski argued that creating a new 64-bit
> Netlink attribute for this would be much more complex than for
> the IFLA_STATS64 support. There was no reply.
>
> Providing a new multiplier/shift parameter would be a simple
> way to extend support for higher rates, and would not break
> existing user space that doesn't require the higher rates.
> I imagine the user would not explicitly specify the multiplier/
> shift parameter, but would just normally specify the desired
> rate, and a newer tc would figure out what multiplier/shift
> to use if a high enough rate demanded it. To maintain user
> space compatibility, the kernel should report back the same
> rate and multiplier/shift it was given, and the newer tc would
> convert it back to the user's originally specified rate. Older
> user space that was fine with the ~34 Gbps rate limitation would
> always have the default multiplier of 1 or shift of 0 bits, and
> would see the exact same unmultiplied/unshifted rate it always
> did.
We already said no to such a hack. Maybe its not clear enough ?
netlink allows us to a proper way, and Thomas Graf explained how we
expect the thing to be done.
Yes, this is not a one liner patch, its a bit more of work, and its how
it will be done when someone does the job.
> I also believe 32 bits of precision is significant enough
> at these higher data rates.
>
This has nothing to do with the issue. Thats an implementation detail in
the kernel. And frankly with 64bit arches we better use 64bit native
fields, instead of adding arbitrary limits.
We are speaking of the netlink message itself and old binaries.
^ permalink raw reply
* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Bill Fink @ 2013-03-13 6:01 UTC (permalink / raw)
To: Thomas Graf; +Cc: Chris Friesen, Eric Dumazet, Vimal, netdev, shemminger
In-Reply-To: <20130312154245.GA13101@casper.infradead.org>
On Tue, 12 Mar 2013, Thomas Graf wrote:
> On 03/12/13 at 08:29am, Chris Friesen wrote:
> > The only problem I see is that you can't set the multiplier with a
> > new tool and then query the rate with old tools.
> >
> > But you're going to run into that problem with the old tools no
> > matter what you do--and not doing anything is a crappy option as
> > well.
> >
> > Some kind of multiplier or shift makes as much sense as anything
> > else. With old tools you get current behaviour, with new tools you
> > can specify a multiplying factor to trade off resolution vs
> > precision.
>
> The introduction of a shift operator or multiplier introduces
> inprecision. I'd much rather see new 64bit Netlink attributes
> that, if present, replace the old rate values and statistics.
>
> You will need to add a new Netlink attribute anyway and we might
> as well transfer the actual rate instead of a multiplier. Just
> like we did with IFLA_STATS64.
The last time this was discussed appears to be (on 2011-03-28):
http://marc.info/?l=linux-netdev&m=130128741907282&w=2
where Maciej Żenczykowski argued that creating a new 64-bit
Netlink attribute for this would be much more complex than for
the IFLA_STATS64 support. There was no reply.
Providing a new multiplier/shift parameter would be a simple
way to extend support for higher rates, and would not break
existing user space that doesn't require the higher rates.
I imagine the user would not explicitly specify the multiplier/
shift parameter, but would just normally specify the desired
rate, and a newer tc would figure out what multiplier/shift
to use if a high enough rate demanded it. To maintain user
space compatibility, the kernel should report back the same
rate and multiplier/shift it was given, and the newer tc would
convert it back to the user's originally specified rate. Older
user space that was fine with the ~34 Gbps rate limitation would
always have the default multiplier of 1 or shift of 0 bits, and
would see the exact same unmultiplied/unshifted rate it always
did.
I also believe 32 bits of precision is significant enough
at these higher data rates.
-Bill
^ permalink raw reply
* Re: atl1c issues on 3.8.2
From: Eric Dumazet @ 2013-03-13 5:57 UTC (permalink / raw)
To: Michael Büsch; +Cc: Eric Dumazet, linux-netdev, David S.Miller
In-Reply-To: <20130312180942.4198e88e@milhouse>
On Tue, 2013-03-12 at 18:09 +0100, Michael Büsch wrote:
> On Tue, 12 Mar 2013 16:45:44 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > On Tue, 2013-03-12 at 16:17 +0100, Michael Büsch wrote:
> > > Hi,
> > >
> > > Starting with 3.8.x scp stalls the atl1c based interface on my Asus Eeepc 1011px.
> > > iperf (for example) does not do that. But after scp stalled the interface,
> > > iperf transfers fail, too.
> >
> > I am pretty sure David stable list contains the needed fix
> >
> > http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
>
> No this didn't fix it.
>
> However, I tried to revert 69b08f62e17439ee3d436faf0b9a7ca6fffb78db again,
> which already caused trouble for me in 3.7
> and this fixed the issue.
>
> So it seems that this still is the same or a related issue that I reported
> for 3.7. I just wrongly stated that the problem was fixed in 3.8, because my
> simple ping test doesn't catch it on 3.8.
>
kmalloc(2000) never had the guarantee that the result would not span two
4K pages.
Apparently the NIC doesn't allow a rx descriptor spanning two 4K pages
or has a particular hardware bug that I can not possibly find myself.
(I don't have atl1c nor any documentation)
atl1c driver authors will need to find the bug and fix the driver.
Drivers that deal with this kind of hardware limitation allocates page
themselves and provide skbs with a fragment to upper stack, or use
build_skb() once the frame is received.
drivers/net/ethernet/intel/igb/igb_main.c is a an example.
Could you try (on net-next tree) different values for the
NETDEV_FRAG_PAGE_MAX_ORDER constant, as it might give to Atheros some
hints ?
(8192 & 16384)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 821c7f4..769fdac 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1844,7 +1844,7 @@ static inline void __skb_queue_purge(struct sk_buff_head *list)
kfree_skb(skb);
}
-#define NETDEV_FRAG_PAGE_MAX_ORDER get_order(32768)
+#define NETDEV_FRAG_PAGE_MAX_ORDER get_order(8192)
#define NETDEV_FRAG_PAGE_MAX_SIZE (PAGE_SIZE << NETDEV_FRAG_PAGE_MAX_ORDER)
#define NETDEV_PAGECNT_MAX_BIAS NETDEV_FRAG_PAGE_MAX_SIZE
^ permalink raw reply related
* Re: [PATCH RFC] ipv6: use stronger hash for reassembly queue hash table
From: Eric Dumazet @ 2013-03-13 5:29 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, yoshfuji, brouer
In-Reply-To: <20130313012715.GE14801@order.stressinduktion.org>
On Wed, 2013-03-13 at 02:27 +0100, Hannes Frederic Sowa wrote:
>
> [PATCH net-next RFC] inet: add max_depth to limit list length in inet_frags hash
>
> This does implement trivial drop for fragments where the hash queue
> is above some limit.
>
> I calculate the limit as follow:
>
> I averaged the folowing formula
>
> max_depth = max_threshold / INETFRAGS_HASHSZ / rounded up (SKB_TRUELEN(0)
> sizeof(struct ipq or struct frag_queue))
>
> to
>
> max_threshold >> 15
>
> So we start with a maximum list length of 128. I think we could halve
> this value to 64, but because I have no real performance data I left it
> at this higher value for now.
>
> This patch does only protect IPv6 (and not netfilter ipv6 defragmentation)
> and will switch off limit checking if max_depth is zero. I'll rewrite
> the check if we agree that this simple solution is the way to go (simple
> drop) and will clamp the minimum value to 1 as soon as I also migrated
> ipv4 and netfilter to the new sysctl handler.
>
> When testing this patch:
>
> Disable netfilter defragmenation for ipv6 on your machine if you test
> this patch, otherwise you won't see the improvment. Machine now runs
> smoothly under fragmentation dos.
>
> Ok if I target this patch for net next time because the hashing changes
> are in there already?
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> include/net/inet_frag.h | 13 +++++++++++++
> net/ipv4/inet_fragment.c | 25 ++++++++++++++++++++++++-
> net/ipv6/reassembly.c | 6 +++++-
> 3 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index 76c3fe5..9ba6ada 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -17,6 +17,7 @@ struct netns_frags {
> int timeout;
> int high_thresh;
> int low_thresh;
> + int max_depth;
> };
>
> struct inet_frag_queue {
> @@ -43,6 +44,11 @@ struct inet_frag_queue {
>
> #define INETFRAGS_HASHSZ 64
>
> +/* max_depth = max_threshold / INETFRAGS_HASHSZ / rounded up (SKB_TRUELEN(0) +
> + * sizeof(struct ipq or struct frag_queue))
> + */
> +#define INETFRAGS_MAXDEPTH_SHIFT 15
> +
This looks like a bit complex to me, as we cant change INETFRAGS_HASHSZ
from userland.
I would just a use a predefined max depth of 128, or even less.
If we have 128 items in a hash chain, then something is really wrong
with our choices.
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index 245ae07..92f1fdd 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -277,6 +277,7 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
> __releases(&f->lock)
> {
> struct inet_frag_queue *q;
> + int depth = 0;
>
> hlist_for_each_entry(q, &f->hash[hash], list) {
> if (q->net == nf && f->match(q, key)) {
> @@ -284,9 +285,31 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
> read_unlock(&f->lock);
> return q;
> }
> + depth++;
> }
> read_unlock(&f->lock);
>
> - return inet_frag_create(nf, f, key);
> + if (!nf->max_depth || depth <= nf->max_depth)
> + return inet_frag_create(nf, f, key);
> + else
> + return NULL;
> }
I would issue a one one time warning in syslog when depth exceeds the
limit.
Thanks !
^ permalink raw reply
* Re: when and where does ep_poll_callback be called ?
From: Eric Dumazet @ 2013-03-13 5:00 UTC (permalink / raw)
To: horseriver; +Cc: netdev
In-Reply-To: <20130114011115.GG2743@debian.localdomain>
On Mon, 2013-01-14 at 09:11 +0800, horseriver wrote:
> hi:
>
>
> I'm studying the epoll module , I can not find ep_poll_callback be called somewhere .
>
>
Thats more a lkml question.
But you can figure it yourself :
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 9fec183..c6e7490 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -928,6 +928,7 @@ static int ep_poll_callback(wait_queue_t *wait, unsigned mode, int sync, void *k
struct epitem *epi = ep_item_from_wait(wait);
struct eventpoll *ep = epi->ep;
+ WARN_ON_ONCE(1);
if ((unsigned long)key & POLLFREE) {
ep_pwq_from_wait(wait)->whead = NULL;
/*
^ permalink raw reply related
* [PATCH] net/core: fixing dst_neigh_lookup/dst_neigh_lookup_skb caused kernel panic
From: Zhouyi Zhou @ 2013-03-13 4:00 UTC (permalink / raw)
To: netdev; +Cc: stable, trivial
when neighbour table is full, dst_neigh_lookup/dst_neigh_lookup_skb
will return -ENOBUFS which
is absolutely non zero, while all the code in kernel which use above
functions assume failure only
on zero return (I did a grep), which will cause kernel panic. (for
example: https://bugzilla.kernel.org/show_bug.cgi?id=54731)
This patch corrects above error with smallest changes to kernel source code.
Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
diff --git a/include/net/dst.h b/include/net/dst.h
index 853cda1..d594f8b 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -413,13 +413,15 @@ static inline int dst_neigh_output(struct
dst_entry *dst, struct neighbour *n,
static inline struct neighbour *dst_neigh_lookup(const struct
dst_entry *dst, const void *daddr)
{
- return dst->ops->neigh_lookup(dst, NULL, daddr);
+ struct neighbour *n = dst->ops->neigh_lookup(dst, NULL, daddr);
+ return IS_ERR(n) ? NULL : n;
}
static inline struct neighbour *dst_neigh_lookup_skb(const struct
dst_entry *dst,
struct sk_buff *skb)
{
- return dst->ops->neigh_lookup(dst, skb, NULL);
+ struct neighbour *n = dst->ops->neigh_lookup(dst, skb, NULL);
+ return IS_ERR(n) ? NULL : n;
}
static inline void dst_link_failure(struct sk_buff *skb)
^ permalink raw reply related
* [PATCH] sctp: Use correct sideffect command in duplicate cookie handling
From: Vlad Yasevich @ 2013-03-13 1:53 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, Vlad Yasevich, Neil Horman
When SCTP is done processing a duplicate cookie chunk, it tries
to delete a newly created association. For that, it has to set
the right association for the side-effect processing to work.
However, when it uses the SCTP_CMD_NEW_ASOC command, that performs
more work then really needed (like hashing the associationa and
assigning it an id) and there is no point to do that only to
delete the association as a next step. In fact, it also creates
an impossible condition where an association may be found by
the getsockopt() call, and that association is empty. This
causes a crash in some sctp getsockopts.
The solution is rather simple. We simply use SCTP_CMD_SET_ASOC
command that doesn't have all the overhead and does exactly
what we need.
Reported-by: Karl Heiss <kheiss@gmail.com>
Tested-by: Karl Heiss <kheiss@gmail.com>
CC: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
---
net/sctp/sm_statefuns.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 5131fcf..de1a013 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2082,7 +2082,7 @@ sctp_disposition_t sctp_sf_do_5_2_4_dupcook(struct net *net,
}
/* Delete the tempory new association. */
- sctp_add_cmd_sf(commands, SCTP_CMD_NEW_ASOC, SCTP_ASOC(new_asoc));
+ sctp_add_cmd_sf(commands, SCTP_CMD_SET_ASOC, SCTP_ASOC(new_asoc));
sctp_add_cmd_sf(commands, SCTP_CMD_DELETE_TCB, SCTP_NULL());
/* Restore association pointer to provide SCTP command interpeter
--
1.7.7.6
^ permalink raw reply related
* [PATCH net-next 4/4] bridge: sync device list when a new uplink is designated
From: Vlad Yasevich @ 2013-03-13 1:45 UTC (permalink / raw)
To: netdev; +Cc: bridge, Vlad Yasevich
In-Reply-To: <1363139126-13396-1-git-send-email-vyasevic@redhat.com>
What a new device is designated as "uplink" sync any device
addresses to it. This allows for dynamic replacement of
uplinks while maintaining the device list.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_netlink.c | 11 +++++++++++
net/bridge/br_sysfs_if.c | 27 ++++++++++++++++++++++++++-
2 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index e08a50e..02f3425 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -326,6 +326,7 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
{
int err;
+ int old_flags = p->flags;
br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
br_set_port_flag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
@@ -349,6 +350,16 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
if (err)
return err;
}
+
+ if ((old_flags ^ p->flags) & BR_UPLINK) {
+ /* UPLINK flag changed on the device. Take care of device
+ * addresses.
+ */
+ if (p->flags & BR_UPLINK)
+ dev_uc_sync(p->dev, p->br->dev);
+ else
+ dev_uc_unsync(p->dev, p->br->dev);
+ }
return 0;
}
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 1f28cd4..d7a89c9 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -158,7 +158,32 @@ static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
-BRPORT_ATTR_FLAG(uplink, BR_UPLINK);
+
+static ssize_t show_uplink(struct net_bridge_port *p, char *buf)
+{
+ return sprintf(buf, "%d\n", !!(p->flags & BR_UPLINK));
+}
+
+static int store_uplink(struct net_bridge_port *p, unsigned long v)
+{
+ unsigned long flags = p->flags;
+
+ if (v)
+ flags |= BR_UPLINK;
+ else
+ flags &= ~BR_UPLINK;
+
+ if (flags != p->flags) {
+ p->flags = flags;
+ if (p->flags & BR_UPLINK)
+ dev_uc_sync(p->dev, p->br->dev);
+ else
+ dev_uc_unsync(p->dev, p->br->dev);
+ br_ifinfo_notify(RTM_NEWLINK, p);
+ }
+ return 0;
+}
+static BRPORT_ATTR(uplink, S_IRUGO | S_IWUSR, show_uplink, store_uplink);
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
--
1.7.7.6
^ permalink raw reply related
* [PATCH net-next 3/4] bridge: Implement IFF_UNICAST_FLT
From: Vlad Yasevich @ 2013-03-13 1:45 UTC (permalink / raw)
To: netdev; +Cc: bridge, Vlad Yasevich
In-Reply-To: <1363139126-13396-1-git-send-email-vyasevic@redhat.com>
Implement IFF_UNICAST_FLT on the bridge. Unicast addresses added
to the bridge device are synched to the uplink devices. This
allows for uplink devices to change while preserving mac assignment.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_device.c | 11 ++++++++++-
net/bridge/br_fdb.c | 6 ++++++
net/bridge/br_if.c | 2 ++
3 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 37f611f..de190fa 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -106,6 +106,15 @@ static int br_dev_open(struct net_device *dev)
static void br_dev_set_multicast_list(struct net_device *dev)
{
+ struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_port *port;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(port, &br->port_list, list) {
+ if (port->flags & BR_UPLINK)
+ dev_uc_sync(port->dev, dev);
+ }
+ rcu_read_unlock();
}
static int br_dev_stop(struct net_device *dev)
@@ -384,7 +393,7 @@ void br_dev_setup(struct net_device *dev)
SET_ETHTOOL_OPS(dev, &br_ethtool_ops);
SET_NETDEV_DEVTYPE(dev, &br_type);
dev->tx_queue_len = 0;
- dev->priv_flags = IFF_EBRIDGE;
+ dev->priv_flags = IFF_EBRIDGE | IFF_UNICAST_FLT;
dev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
NETIF_F_GSO_MASK | NETIF_F_HW_CSUM | NETIF_F_LLTX |
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index b0812c9..ef7b51e 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -677,6 +677,9 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
struct net_port_vlans *pv;
unsigned short vid = VLAN_N_VID;
+ if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
+ return ndo_dflt_fdb_add(ndm, tb, dev, addr, nlh_flags);
+
if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE))) {
pr_info("bridge: RTM_NEWNEIGH with invalid state %#x\n", ndm->ndm_state);
return -EINVAL;
@@ -774,6 +777,9 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
struct net_port_vlans *pv;
unsigned short vid = VLAN_N_VID;
+ if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
+ return ndo_dflt_fdb_del(ndm, tb, dev, addr);
+
if (tb[NDA_VLAN]) {
if (nla_len(tb[NDA_VLAN]) != sizeof(unsigned short)) {
pr_info("bridge: RTM_NEWNEIGH with invalid vlan\n");
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 02b4440..5196a79 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -145,6 +145,8 @@ static void del_nbp(struct net_bridge_port *p)
nbp_vlan_flush(p);
br_fdb_delete_by_port(br, p, 1);
+ if (p->flags & BR_UPLINK)
+ dev_uc_unsync(dev, br->dev);
list_del_rcu(&p->list);
--
1.7.7.6
^ permalink raw reply related
* [PATCH net-next 2/4] bridge: Allow an ability to designate an uplink port
From: Vlad Yasevich @ 2013-03-13 1:45 UTC (permalink / raw)
To: netdev; +Cc: bridge, Vlad Yasevich
In-Reply-To: <1363139126-13396-1-git-send-email-vyasevic@redhat.com>
Allow a ports to be designated as uplink. Multiple ports
may be designated as uplinks and they will be kept in a
list.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/uapi/linux/if_link.h | 1 +
net/bridge/br_netlink.c | 2 ++
net/bridge/br_private.h | 1 +
net/bridge/br_sysfs_if.c | 2 ++
4 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index c4edfe1..b9444e9 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -220,6 +220,7 @@ enum {
IFLA_BRPORT_GUARD, /* bpdu guard */
IFLA_BRPORT_PROTECT, /* root port protection */
IFLA_BRPORT_FAST_LEAVE, /* multicast fast leave */
+ IFLA_BRPORT_UPLINK, /* uplink port */
__IFLA_BRPORT_MAX
};
#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 27aa3ee..e08a50e 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -283,6 +283,7 @@ static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
[IFLA_BRPORT_MODE] = { .type = NLA_U8 },
[IFLA_BRPORT_GUARD] = { .type = NLA_U8 },
[IFLA_BRPORT_PROTECT] = { .type = NLA_U8 },
+ [IFLA_BRPORT_UPLINK] = { .type = NLA_U8 },
};
/* Change the state of the port and notify spanning tree */
@@ -329,6 +330,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
br_set_port_flag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
br_set_port_flag(p, tb, IFLA_BRPORT_FAST_LEAVE, BR_MULTICAST_FAST_LEAVE);
+ br_set_port_flag(p, tb, IFLA_BRPORT_UPLINK, BR_UPLINK);
if (tb[IFLA_BRPORT_COST]) {
err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 4a0fa29..44ae584 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -156,6 +156,7 @@ struct net_bridge_port
#define BR_BPDU_GUARD 0x00000002
#define BR_ROOT_BLOCK 0x00000004
#define BR_MULTICAST_FAST_LEAVE 0x00000008
+#define BR_UPLINK 0x00000010
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
u32 multicast_startup_queries_sent;
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index a1ef1b6..1f28cd4 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -158,6 +158,7 @@ static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
+BRPORT_ATTR_FLAG(uplink, BR_UPLINK);
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -195,6 +196,7 @@ static const struct brport_attribute *brport_attrs[] = {
&brport_attr_hairpin_mode,
&brport_attr_bpdu_guard,
&brport_attr_root_block,
+ &brport_attr_uplink,
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
&brport_attr_multicast_router,
&brport_attr_multicast_fast_leave,
--
1.7.7.6
^ permalink raw reply related
* [PATCH net-next 1/4] bridge: Add sysfs interface to control promisc mode
From: Vlad Yasevich @ 2013-03-13 1:45 UTC (permalink / raw)
To: netdev; +Cc: bridge, Vlad Yasevich
In-Reply-To: <1363139126-13396-1-git-send-email-vyasevic@redhat.com>
Add an sysfs interface to turn promiscusous mode on and off on
the bridge. By default all interfaces in the bridge
are in promisc mode. When promisc mode is turned off, it is
turned off on all bridge ports and the bridge turns on IFF_ALLMULTI
to handle multicast traffic.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_device.c | 41 +++++++++++++++++++++++++++++++++++++++++
net/bridge/br_if.c | 22 +++++++++++++++++-----
net/bridge/br_private.h | 2 ++
net/bridge/br_sysfs_br.c | 17 +++++++++++++++++
4 files changed, 77 insertions(+), 5 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index d5f1d3f..37f611f 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -185,6 +185,46 @@ static int br_set_mac_address(struct net_device *dev, void *p)
return 0;
}
+int br_set_promisc(struct net_bridge *br, unsigned long val)
+{
+ struct net_bridge_port *p;
+
+ if (!rtnl_trylock())
+ return restart_syscall();
+
+ spin_lock_bh(&br->lock);
+
+ if (val) {
+ if (br->promisc_enabled)
+ goto unlock;
+
+ br->promisc_enabled = 1;
+ /* For each port on the bridge, turn on promisc mode and
+ * turn off ALLMULTI to handle multicast traffic.
+ */
+ list_for_each_entry(p, &br->port_list, list) {
+ dev_set_promiscuity(p->dev, 1);
+ dev_set_allmulti(p->dev, -1);
+ }
+
+ } else {
+ if (!br->promisc_enabled)
+ goto unlock;
+
+ br->promisc_enabled = 0;
+ /* For each port on the bridge, turn off promisc mode */
+ list_for_each_entry(p, &br->port_list, list) {
+ dev_set_allmulti(p->dev, 1);
+ dev_set_promiscuity(p->dev, -1);
+ }
+ }
+unlock:
+ spin_unlock_bh(&br->lock);
+ rtnl_unlock();
+ return 0;
+}
+
+
static void br_getinfo(struct net_device *dev, struct ethtool_drvinfo *info)
{
strlcpy(info->driver, "bridge", sizeof(info->driver));
@@ -371,6 +411,7 @@ void br_dev_setup(struct net_device *dev)
br->bridge_hello_time = br->hello_time = 2 * HZ;
br->bridge_forward_delay = br->forward_delay = 15 * HZ;
br->ageing_time = 300 * HZ;
+ br->promisc_enabled = 1;
br_netfilter_rtable_init(br);
br_stp_timer_init(br);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index ef1b914..02b4440 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -132,7 +132,10 @@ static void del_nbp(struct net_bridge_port *p)
sysfs_remove_link(br->ifobj, p->dev->name);
- dev_set_promiscuity(dev, -1);
+ if (br->promisc_enabled)
+ dev_set_promiscuity(dev, -1);
+ else
+ dev_set_allmulti(dev, -1);
spin_lock_bh(&br->lock);
br_stp_disable_port(p);
@@ -350,9 +353,15 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
call_netdevice_notifiers(NETDEV_JOIN, dev);
- err = dev_set_promiscuity(dev, 1);
- if (err)
- goto put_back;
+ if (br->promisc_enabled) {
+ err = dev_set_promiscuity(dev, 1);
+ if (err)
+ goto put_back;
+ } else {
+ err = dev_set_allmulti(dev, 1);
+ if (err)
+ goto put_back;
+ }
err = kobject_init_and_add(&p->kobj, &brport_ktype, &(dev->dev.kobj),
SYSFS_BRIDGE_PORT_ATTR);
@@ -414,7 +423,10 @@ err2:
kobject_put(&p->kobj);
p = NULL; /* kobject_put frees */
err1:
- dev_set_promiscuity(dev, -1);
+ if (br->promisc_enabled)
+ dev_set_promiscuity(dev, -1);
+ else
+ dev_set_allmulti(dev, -1);
put_back:
dev_put(dev);
kfree(p);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6d314c4..4a0fa29 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -277,6 +277,7 @@ struct net_bridge
struct timer_list topology_change_timer;
struct timer_list gc_timer;
struct kobject *ifobj;
+ u8 promisc_enabled;
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
u8 vlan_enabled;
struct net_port_vlans __rcu *vlan_info;
@@ -327,6 +328,7 @@ extern void br_dev_setup(struct net_device *dev);
extern void br_dev_delete(struct net_device *dev, struct list_head *list);
extern netdev_tx_t br_dev_xmit(struct sk_buff *skb,
struct net_device *dev);
+extern int br_set_promisc(struct net_bridge *br, unsigned long val);
#ifdef CONFIG_NET_POLL_CONTROLLER
static inline struct netpoll_info *br_netpoll_info(struct net_bridge *br)
{
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 8baa9c0..5489219 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -710,6 +710,22 @@ static DEVICE_ATTR(vlan_filtering, S_IRUGO | S_IWUSR,
show_vlan_filtering, store_vlan_filtering);
#endif
+static ssize_t show_promisc(struct device *d,
+ struct device_attribute *attr, char *buf)
+{
+ struct net_bridge *br = to_bridge(d);
+ return sprintf(buf, "%d\n", br->promisc_enabled);
+}
+
+static ssize_t store_promisc(struct device *d,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return store_bridge_parm(d, buf, len, br_set_promisc);
+}
+static DEVICE_ATTR(promisc, S_IRUGO | S_IWUSR, show_promisc,
+ store_promisc);
+
static struct attribute *bridge_attrs[] = {
&dev_attr_forward_delay.attr,
&dev_attr_hello_time.attr,
@@ -753,6 +769,7 @@ static struct attribute *bridge_attrs[] = {
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
&dev_attr_vlan_filtering.attr,
#endif
+ &dev_attr_promisc.attr,
NULL
};
--
1.7.7.6
^ permalink raw reply related
* [PATCH net-next 0/4] Allow bridge to function in non-promisc mode
From: Vlad Yasevich @ 2013-03-13 1:45 UTC (permalink / raw)
To: netdev; +Cc: bridge, Vlad Yasevich
The series adds an ability for the bridge to function in non-promiscuous mode.
We do it in 3 steps.
First we add an interface to palce the switch into non-promisc mode. In
this mode, all port of the switch turn promisc off and turn on IFF_ALLMULTI
to continue handling multicast traffic.
Second we add an ability to designate a bridge port as uplink.
Third we add IFF_UNICAST_FLT support to the bridge and sync all unicast
HW addresses to the uplink ports.
Default bridge operation continues to remain "promiscuous". The new
functionality has to be enabled via sysfs (similar to other bridge extensions).
The uplink mode is implemented as a flag on a bridge port. The api to
change that flag follows the existing api to enable/disable other existing
flags.
Changes since rfc v2:
* Sync/unsync address on uplink upon the uplink flag change. This allows
for uplink replacements without loss of addresses.
Changes since rfc v1:
* Fixed submit log
* Simplifyied uplink logic. Uplink is now a flag per port. This removes the
need for a separate list.
* Clean-up hw list once the port has been removed.
Vlad Yasevich (4):
bridge: Add sysfs interface to control promisc mode
bridge: Allow an ability to designate an uplink port
bridge: Implement IFF_UNICAST_FLT
bridge: sync device list when a new uplink is designated
include/uapi/linux/if_link.h | 1 +
net/bridge/br_device.c | 52 +++++++++++++++++++++++++++++++++++++++++-
net/bridge/br_fdb.c | 6 +++++
net/bridge/br_if.c | 24 +++++++++++++++----
net/bridge/br_netlink.c | 13 ++++++++++
net/bridge/br_private.h | 3 ++
net/bridge/br_sysfs_br.c | 17 +++++++++++++
net/bridge/br_sysfs_if.c | 27 +++++++++++++++++++++
8 files changed, 137 insertions(+), 6 deletions(-)
--
1.7.7.6
^ permalink raw reply
* Re: [PATCH] sctp: optimize searching the active path for tsns
From: Vlad Yasevich @ 2013-03-13 1:43 UTC (permalink / raw)
To: Neil Horman; +Cc: linux-sctp, Xufeng Zhang, davem, netdev
In-Reply-To: <20130313012006.GA6958@hmsreliant.think-freely.org>
On 03/12/2013 09:20 PM, Neil Horman wrote:
> On Tue, Mar 12, 2013 at 05:01:50PM -0400, Vlad Yasevich wrote:
>> Hi Neil
>>
>> On 03/12/2013 01:29 PM, Neil Horman wrote:
>>> SCTP currently attempts to optimize the search for tsns on a transport by first
>>> checking the active_path, then searching alternate transports. This operation
>>> however is a bit convoluted, as we explicitly search the active path, then serch
>>> all other transports, skipping the active path, when its detected. Lets
>>> optimize this by preforming a move to front on the transport_addr_list every
>>> time the active_path is changed. The active_path changes occur in relatively
>>> non-critical paths, and doing so allows us to just search the
>>> transport_addr_list in order, avoiding an extra conditional check in the
>>> relatively hot tsn lookup path. This also happens to fix a bug where we break
>>> out of the for loop early in the tsn lookup.
>>>
>>> CC: Xufeng Zhang <xufengzhang.main@gmail.com>
>>> CC: vyasevich@gmail.com
>>> CC: davem@davemloft.net
>>> CC: netdev@vger.kernel.org
>>> ---
>>> net/sctp/associola.c | 31 ++++++++++++-------------------
>>> 1 file changed, 12 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>> index 43cd0dd..7af96b3 100644
>>> --- a/net/sctp/associola.c
>>> +++ b/net/sctp/associola.c
>>> @@ -513,8 +513,11 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
>>> * user wants to use this new path.
>>> */
>>> if ((transport->state == SCTP_ACTIVE) ||
>>> - (transport->state == SCTP_UNKNOWN))
>>> + (transport->state == SCTP_UNKNOWN)) {
>>> + list_del_rcu(&transport->transports);
>>> + list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
>>> asoc->peer.active_path = transport;
>>> + }
>>
>> What would happen if at the same time someone is walking the list
>> through the proc interfaces?
>>
>> Since you are effectively changing the .next pointer, I think it is
>> possible to get a duplicate transport output essentially corrupting
>> the output.
>>
> It would be the case, but you're using the rcu variants of the list_add macros
> at all the points where we modify the list (some of which we do at call sites
> right before we call set_primary, see sctp_assoc_add_peer, where
> list_add_tail_rcu also modifies a next pointer). So if this is a problem, its a
> problem without this patch. In fact looking at it, our list access to
> transport_addr_list is broken, as we use rcu apis to modify the list but non-rcu
> apis to traverse the list. I'll need to fix that first.
As long as we are under lock, we don't need rcu variants for traversal.
The traversals done by the sctp_seq_ functions already use correct rcu
variants.
I don't see this as a problem right now since we either delete the
transport, or add it. We don't move it to a new location in the list.
With the move, what could happen is that while sctp_seq_ is printing
a transport, you might move it to another spot, and the when you grab
the .next pointer on the next iteration, it points to a completely
different spot.
-vlad
>
>> Personally, I don't think that this particular case is worth the
>> optimization since we are trying to optimize a TSN search that only
>> happens when ECNE chunk is received. You say that it is a hot path.
>> Is ECNE really such a common occurrence?
>>
> Not particularly, but I'm guessing its more used than the path that switches the
> active path. Plus its just more elegant, modifying the list so we don't have to
> have two identical for loops in the lookup_tsn function, plus a check for the
> transport that we already checked.
>
>> Additionally, I don't think this is really an optimization as the
>> current and new code do exactly the same thing:
>> 1) look at active path
>> 2) look at the rest of the paths
>>
> Thats not the optimization, its trading where we do work in the interests of
> making the more used path faster (we drop an if condition, and remove some
> duplicated code).
>
>> This just makes cleaner code at the expense of list shuffling.
>>
> Yes, that sounds like a good trade to me. But as noted above, we probably need
> to fix the transport_addr_list access first.
> Neil
>
>> -vlad
>>>
>>> /*
>>> * SFR-CACC algorithm:
>>> @@ -964,6 +967,10 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
>>> }
>>>
>>> /* Set the active and retran transports. */
>>> + if (asoc->peer.active_path != first) {
>>> + list_del_rcu(first);
>>> + list_add_rcu(first, &asoc->peer.transport_addr_list);
>>> + }
>>> asoc->peer.active_path = first;
>>> asoc->peer.retran_path = second;
>>> }
>>> @@ -1040,7 +1047,6 @@ struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
>>> struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
>>> __u32 tsn)
>>> {
>>> - struct sctp_transport *active;
>>> struct sctp_transport *match;
>>> struct sctp_transport *transport;
>>> struct sctp_chunk *chunk;
>>> @@ -1057,29 +1063,16 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
>>> * The general strategy is to search each transport's transmitted
>>> * list. Return which transport this TSN lives on.
>>> *
>>> - * Let's be hopeful and check the active_path first.
>>> - * Another optimization would be to know if there is only one
>>> - * outbound path and not have to look for the TSN at all.
>>> + * Note, that sctp_assoc_set_primary does a move to front operation
>>> + * on the active_path transport, so this code implicitly checks
>>> + * the active_path first, as we most commonly expect to find our TSN
>>> + * there.
>>> *
>>> */
>>>
>>> - active = asoc->peer.active_path;
>>> -
>>> - list_for_each_entry(chunk, &active->transmitted,
>>> - transmitted_list) {
>>> -
>>> - if (key == chunk->subh.data_hdr->tsn) {
>>> - match = active;
>>> - goto out;
>>> - }
>>> - }
>>> -
>>> - /* If not found, go search all the other transports. */
>>> list_for_each_entry(transport, &asoc->peer.transport_addr_list,
>>> transports) {
>>>
>>> - if (transport == active)
>>> - break;
>>> list_for_each_entry(chunk, &transport->transmitted,
>>> transmitted_list) {
>>> if (key == chunk->subh.data_hdr->tsn) {
>>>
>>
>>
^ 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