Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
From: Rob Herring @ 2016-10-10 15:15 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Corentin Labbe, mark.rutland-5wv7dgnIgG8, wens-jdAy2FN1RRM,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, andrew-g2DYL2Zd6BY,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161010123023.GG3462@lukather>

On Mon, Oct 10, 2016 at 02:30:23PM +0200, Maxime Ripard wrote:
> On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > Allwinner sun8i-emac driver.
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  .../bindings/net/allwinner,sun8i-emac.txt          | 70 ++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > new file mode 100644
> > index 0000000..92e4ef3b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > @@ -0,0 +1,70 @@
> > +* Allwinner sun8i EMAC ethernet controller
> > +
> > +Required properties:
> > +- compatible: should be one of the following string:
> > +		"allwinner,sun8i-a83t-emac"
> > +		"allwinner,sun8i-h3-emac"
> > +		"allwinner,sun50i-a64-emac"
> > +- reg: address and length of the register for the device.
> > +- syscon: A phandle to the syscon of the SoC
> > +- interrupts: interrupt for the device
> > +- clocks: A phandle to the reference clock for this device
> > +- clock-names: should be "ahb"
> > +- resets: A phandle to the reset control for this device
> > +- reset-names: should be "ahb"
> > +- phy-mode: See ethernet.txt
> > +- phy-handle: See ethernet.txt
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. Default is 0)
> > +Both delay properties does not have units, there are arbitrary value.
> > +The TX/RX clock delay chain settings are board specific and could be found
> > +in vendor FEX files.
> > +
> > +Optional properties for "allwinner,sun8i-h3-emac":
> > +- allwinner,leds-active-low: EPHY LEDs are active low
> > +
> > +Required child node of emac:
> > +- mdio bus node: should be named mdio
> > +
> > +Required properties of the mdio node:
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +The device node referenced by "phy" or "phy-handle" should be a child node
> > +of the mdio node. See phy.txt for the generic PHY bindings.
> > +
> > +Required properties of the phy node with "allwinner,sun8i-h3-emac":
> > +- clocks: an extra phandle to the reference clock for the EPHY
> > +- resets: an extra phandle to the reset control for the EPHY
> > +
> > +Example:
> > +
> > +emac: ethernet@01c0b000 {
> > +	compatible = "allwinner,sun8i-h3-emac";
> > +	syscon = <&syscon>;
> > +	reg = <0x01c0b000 0x104>;
> > +	interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> > +	resets = <&ccu RST_BUS_EMAC>;
> > +	reset-names = "ahb";
> > +	clocks = <&ccu CLK_BUS_EMAC>;
> > +	clock-names = "ahb";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	phy = <&int_mii_phy>;
> > +	phy-mode = "mii";
> > +	allwinner,leds-active-low;
> > +	mdio: mdio {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		int_mii_phy: ethernet-phy@1 {
> > +			reg = <1>;
> > +			clocks = <&ccu CLK_BUS_EPHY>;
> > +			resets = <&ccu RST_BUS_EPHY>;
> 
> That works for me, let's see how the DT maintainers feel about it.

The phy should have a compatible string since you have extra properties.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH kernel v2] PCI: Enable access to custom VPD for Chelsio devices (cxgb3)
From: Alexander Duyck @ 2016-10-10 15:23 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Bjorn Helgaas, Netdev, Santosh Raspatur,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <1475126512-10722-1-git-send-email-aik@ozlabs.ru>

On Wed, Sep 28, 2016 at 10:21 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> There is at least one Chelsio 10Gb card which uses VPD area to store
> some custom blocks (example below). However pci_vpd_size() returns
> the length of the first block only assuming that there can be only
> one VPD "End Tag" and VFIO blocks access beyond that offset
> (since 4e1a63555) which leads to the situation when the guest "cxgb3"
> driver fails to probe the device. The host system does not have this
> problem as the drives accesses the config space directly without
> pci_read_vpd()/...
>
> This adds a quirk to override the VPD size to a bigger value.
> The maximum size is taken from EEPROMSIZE in
> drivers/net/ethernet/chelsio/cxgb3/common.h. We do not read the tag
> as the cxgb3 driver does as the driver supports writing to EEPROM/VPD
> and when it writes, it only checks for 8192 bytes boundary. The quirk
> is registerted for all devices supported by the cxgb3 driver.
>
> This adds a quirk to the PCI layer (not to the cxgb3 driver) as
> the cxgb3 driver itself accesses VPD directly and the problem only exists
> with the vfio-pci driver (when cxgb3 is not running on the host and
> may not be even loaded) which blocks accesses beyond the first block
> of VPD data. However vfio-pci itself does not have quirks mechanism so
> we add it to PCI.
>
> Tested on:
> Ethernet controller [0200]: Chelsio Communications Inc T310 10GbE Single Port Adapter [1425:0030]
>
> This is its VPD:
> 0000 Large item 42 bytes; name 0x2 Identifier String
>         b'10 Gigabit Ethernet-SR PCI Express Adapter'
>         #00 [EC] len=7: b'D76809 '
>         #0a [FN] len=7: b'46K7897'
>         #14 [PN] len=7: b'46K7897'
>         #1e [MN] len=4: b'1037'
>         #25 [FC] len=4: b'5769'
>         #2c [SN] len=12: b'YL102035603V'
>         #3b [NA] len=12: b'00145E992ED1'
>
> 0c00 Large item 16 bytes; name 0x2 Identifier String
>         b'S310E-SR-X      '
> 0c13 Large item 234 bytes; name 0x10
>         #00 [PN] len=16: b'TBD             '
>         #13 [EC] len=16: b'110107730D2     '
>         #26 [SN] len=16: b'97YL102035603V  '
>         #39 [NA] len=12: b'00145E992ED1'
>         #48 [V0] len=6: b'175000'
>         #51 [V1] len=6: b'266666'
>         #5a [V2] len=6: b'266666'
>         #63 [V3] len=6: b'2000  '
>         #6c [V4] len=2: b'1 '
>         #71 [V5] len=6: b'c2    '
>         #7a [V6] len=6: b'0     '
>         #83 [V7] len=2: b'1 '
>         #88 [V8] len=2: b'0 '
>         #8d [V9] len=2: b'0 '
>         #92 [VA] len=2: b'0 '
>         #97 [RV] len=80: b's\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
> 0d00 Large item 252 bytes; name 0x11
>         #00 [VC] len=16: b'122310_1222 dp  '
>         #13 [VD] len=16: b'610-0001-00 H1\x00\x00'
>         #26 [VE] len=16: b'122310_1353 fp  '
>         #39 [VF] len=16: b'610-0001-00 H1\x00\x00'
>         #4c [RW] len=173: b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
> 0dff Small item 0 bytes; name 0xf End Tag
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v2:
> * used pci_set_vpd_size() helper
> * added explicit list of IDs from cxgb3 driver
> * added a note in the commit log why the quirk is not in cxgb3
> ---
>  drivers/pci/quirks.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 44e0ff3..b22fce5 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3243,6 +3243,28 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PORT_RIDGE,
>                         quirk_thunderbolt_hotplug_msi);
>
> +static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
> +{
> +       if (!dev->vpd)
> +               return;
> +
> +       pci_set_vpd_size(dev, max_t(unsigned int, dev->vpd->len, 8192));

What is the point of the max_t?  From what I can tell you aren't
writing 8192, you will always end up writing 32K since that is the
starting value for dev->vpd->len assuming there have yet to be any
reads.

What you may want to do instead is just modify the pci_vpd_size
function you can use that in your quirk, and modify it so that you can
pass an offset  Then you could just start it with an offset of 0x0c00
and have it read to get the exact size of the region covered in this
second block of the VPD.

> +}
> +
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x20, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x21, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x22, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x23, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x24, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x25, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x26, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x30, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x31, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x32, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x35, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x36, quirk_chelsio_extend_vpd);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x37, quirk_chelsio_extend_vpd);
> +
>  #ifdef CONFIG_ACPI
>  /*
>   * Apple: Shutdown Cactus Ridge Thunderbolt controller.
> --
> 2.5.0.rc3
>

^ permalink raw reply

* Re: [PATCH net 2/2] conntrack: enable to tune gc parameters
From: Nicolas Dichtel @ 2016-10-10 15:24 UTC (permalink / raw)
  To: Florian Westphal; +Cc: davem, pablo, netdev, netfilter-devel
In-Reply-To: <20161010140424.GB21057@breakpoint.cc>

Le 10/10/2016 à 16:04, Florian Westphal a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>> After commit b87a2f9199ea ("netfilter: conntrack: add gc worker to remove
>> timed-out entries"), netlink conntrack deletion events may be sent with a
>> huge delay. It could be interesting to let the user tweak gc parameters
>> depending on its use case.
> 
> Hmm, care to elaborate?
> 
> I am not against doing this but I'd like to hear/read your use case.
> 
> The expectation is that in almot all cases eviction will happen from
> packet path.  The gc worker is jusdt there for case where a busy system
> goes idle.
It was precisely that case. After a period of activity, the event is sent a long
time after the timeout. If the router does not manage a lot of flows, why not
trying to parse more entries instead of the default 1/64 of the table?
In fact, I don't understand why using GC_MAX_BUCKETS_DIV instead of using always
GC_MAX_BUCKETS whatever the size of the table is.

> 
>> +nf_conntrack_gc_max_evicts - INTEGER
>> +	The maximum number of entries to be evicted during a run of gc.
>> +	This sysctl is only writeable in the initial net namespace.
> 
> Hmmm, do you have any advice on sizing this one?
In fact, no ;-)
I really hesitate to expose the four values or just a subset. My goal was also
to get feedback. I can remove this one.

> 
> I think a better change might be (instead of adding htis knob) to
> resched the gc worker for immediate re-executaion in case the entire
> "budget" was used.  What do you think?
Even if it's not directly related to my problem, I think it's a good idea.

> 
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -983,7 +983,7 @@ static void gc_worker(struct work_struct *work)
>                 return;
>  
>         ratio = scanned ? expired_count * 100 / scanned : 0;
> -       if (ratio >= 90)
> +       if (ratio >= 90 || expired_count == GC_MAX_EVICTS)
>                 next_run = 0;
> 

^ permalink raw reply

* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Sergey Senozhatsky @ 2016-10-10 15:30 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andy Lutomirski, David S. Miller, Johannes Berg, linux-wireless,
	netdev, linux-kernel, Sergey Senozhatsky, linux-next,
	Stephen Rothwell
In-Reply-To: <20161010150358.GA514@swordfish>

Cc Andy

Andy, can this be related to CONFIG_VMAP_STACK?

On (10/11/16 00:03), Sergey Senozhatsky wrote:
> Hello,
> 
> current -git kills my system. adding
> 
> 	if (!virt_addr_valid(&aad[2])) {
> 		WARN_ON(1);
> 		return -EINVAL;
> 	}
> 
> to ieee80211_aes_ccm_decrypt() given the following backtrace
> 
>  WARNING: CPU: 5 PID: 252 at net/mac80211/aes_ccm.c:77 ieee80211_aes_ccm_decrypt+0xc8/0x197
>  CPU: 5 PID: 252 Comm: irq/29-iwlwifi Tainted: G        W       4.8.0-next-20161010-dbg-00007-g79797e9-dirty #88
>   ffffc90000413638 ffffffff811ff0e3 0000000000000000 0000000000000000
>   ffffc90000413678 ffffffff8103fe91 0000004d000001c8 1ffff920000826d3
>   ffff88040fc526d8 0000000000000008 ffffc90000413978 ffffc9000041397a
>  Call Trace:
>   [<ffffffff811ff0e3>] dump_stack+0x4f/0x65
>   [<ffffffff8103fe91>] __warn+0xc2/0xdd
>   [<ffffffff8103ff1c>] warn_slowpath_null+0x1d/0x1f
>   [<ffffffff8142aaa5>] ieee80211_aes_ccm_decrypt+0xc8/0x197
>   [<ffffffff810ed595>] ? __put_page+0x3c/0x3f
>   [<ffffffff8131fa42>] ? put_page+0x4a/0x62
>   [<ffffffff813218d3>] ? __pskb_pull_tail+0x1e8/0x279
>   [<ffffffff8141a7dc>] ? ccmp_special_blocks.isra.5+0x51/0x12d
>   [<ffffffff8141b226>] ieee80211_crypto_ccmp_decrypt+0x1ba/0x221
>   [<ffffffff81432e80>] ieee80211_rx_handlers+0x52a/0x19c2
>   [<ffffffff81070000>] ? start_dl_timer+0xa8/0xb4
>   [<ffffffff8107462d>] ? put_lock_stats.isra.24+0xe/0x20
>   [<ffffffff8108ebec>] ? del_timer+0x57/0x61
>   [<ffffffff814351a8>] ieee80211_prepare_and_rx_handle+0xcd6/0xd2a
>   [<ffffffff810742a5>] ? local_clock+0x10/0x12
>   [<ffffffff8107642b>] ? __lock_acquire.isra.31+0x202/0x57e
>   [<ffffffff8143207b>] ? rcu_read_unlock+0x23/0x23
>   [<ffffffff81066e77>] ? sched_clock_cpu+0x17/0xc6
>   [<ffffffff814357ab>] ieee80211_rx_napi+0x5af/0x698
>   [<ffffffff810742c0>] ? get_lock_stats+0x19/0x50
>   [<ffffffff8107462d>] ? put_lock_stats.isra.24+0xe/0x20
>   [<ffffffffa023aaa9>] iwl_mvm_rx_rx_mpdu+0x5ab/0x60c [iwlmvm]
>   [<ffffffff810742c0>] ? get_lock_stats+0x19/0x50
>   [<ffffffffa0235c80>] iwl_mvm_rx+0x45/0x69 [iwlmvm]
>   [<ffffffffa01a989e>] iwl_pcie_rx_handle+0x478/0x584 [iwlwifi]
>   [<ffffffffa01aaafd>] iwl_pcie_irq_handler+0x39c/0x52d [iwlwifi]
>   [<ffffffff81080824>] ? irq_finalize_oneshot+0xa7/0xa7
>   [<ffffffff81080841>] irq_thread_fn+0x1d/0x34
>   [<ffffffff81080ab5>] irq_thread+0xe6/0x1bb
>   [<ffffffff8108093a>] ? wake_threads_waitq+0x2c/0x2c
>   [<ffffffff810809cf>] ? irq_thread_dtor+0x95/0x95
>   [<ffffffff81059d79>] kthread+0xc6/0xce
>   [<ffffffff8107462d>] ? put_lock_stats.isra.24+0xe/0x20
>   [<ffffffff81059cb3>] ? __list_del_entry+0x22/0x22
>   [<ffffffff814669d2>] ret_from_fork+0x22/0x30
>  ---[ end trace 94da6d4698b938b2 ]---

	-ss

^ permalink raw reply

* Re: Accelerated receive flow steering (aRFS) for UDP
From: Rick Jones @ 2016-10-10 16:08 UTC (permalink / raw)
  To: Eric Dumazet, Chopra, Manish
  Cc: netdev@vger.kernel.org, maorg@mellanox.com, tom@herbertland.com
In-Reply-To: <1476052402.28155.277.camel@edumazet-glaptop3.roam.corp.google.com>

On 10/09/2016 03:33 PM, Eric Dumazet wrote:
> OK, I am adding/CC Rick Jones, netperf author, since it seems a netperf
> bug, not a kernel one.
>
> I believe I already mentioned fact that "UDP_STREAM -- -N" was not doing
> a connect() on the receiver side.

I can confirm that the receive side of the netperf omni path isn't 
trying to connect UDP datagrams.  I will see what I can put together.

happy benchmarking,

rick jones
rick.jones2@hpe.com

^ permalink raw reply

* Re: slab corruption with current -git
From: Linus Torvalds @ 2016-10-10 16:15 UTC (permalink / raw)
  To: David Miller
  Cc: Aaron Conole, Florian Westphal, Al Viro, Andrew Morton,
	Jens Axboe, Theodore Ts'o, Christoph Lameter,
	Pablo Neira Ayuso, Linux Kernel Mailing List, linux-fsdevel,
	Network Development, NetFilter
In-Reply-To: <20161010.042401.637964142015887598.davem@davemloft.net>

On Mon, Oct 10, 2016 at 1:24 AM, David Miller <davem@davemloft.net> wrote:
>
> So I've been reviewing this patch and it looks fine, but I also want
> to figure out what is actually causing the OOPS and I can't spot it
> yet.

Yeah, I'm not actually sure the old linked list implementation is
buggy - it might just be ugly. I tried to follow the old code, and I
couldn't.

So the patch I sent out was a combination of "that's not how you
should do singly linked lists" and "those special cases make me
worry". In particular, the old code really ended up doing odd things
in the "can't find entry" case, because it would exit the loop with a
non-NULL 'entry' just because the next entry was NULL..

> One possible way to see that oops is to free the head entry of the
> chain without unlinking it.  The next unregister will dereference a
> POISON pointer.
>
> Actually...
>
> The POISON value comes not from a hook entry, but from the array of
> pointers in the per-netns datastructure.
>
> This means that the netns is possibly getting freed up before we
> unregister the netfilter hooks.

That is certainly one possible explanation for it, yes. However, I
didn't think that part had changed, had it?

The other thing I find a bit odd in that new single-linked list code is this:

 - nf_hook_slow():
        ...
        RCU_INIT_POINTER(state->hook_entries, entry);

which makes me worried..  It copies the head entry of the list, and
maybe it will then (later) end up being used stale. I don't know. But
it looks a bit iffy.

              Linus

^ permalink raw reply

* Re: [PATCH net] net: add recursion limit to GRO
From: Sabrina Dubroca @ 2016-10-10 16:20 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Hannes Frederic Sowa, Jiri Benc
In-Reply-To: <1476108236.28155.299.camel@edumazet-glaptop3.roam.corp.google.com>

Hi Eric,

2016-10-10, 07:03:56 -0700, Eric Dumazet wrote:
> On Mon, 2016-10-10 at 15:43 +0200, Sabrina Dubroca wrote:
> > Currently, GRO can do unlimited recursion through the gro_receive
> > handlers.  This was fixed for tunneling protocols by limiting tunnel GRO
> > to one level with encap_mark, but both VLAN and TEB still have this
> > problem.  Thus, the kernel is vulnerable to a stack overflow, if we
> > receive a packet composed entirely of VLAN headers.
> > 
> > This patch adds a recursion counter to the GRO layer to prevent stack
> > overflow.  When a gro_receive function hits the recursion limit, GRO is
> > aborted for this skb and it is processed normally.
> > 
> > Thanks to Vladimír Beneš <vbenes@redhat.com> for the initial bug report.
> 
> 
> Hi Sabrina
> 
> Have you considered using a per cpu counter ?
>
> It might be cheaper than using a 4-bit field in skb.

I thought about it, but this looked a bit simpler. I can try some
benchmarking tomorrow.


> Really this counter does not need to be stored in skb. GRO already uses
> way too much space in skb->cb[]

For net-next I'm working on turning GRO into a loop, which would
eliminate these few bits.


> Also please add appropriate unlikely() clauses, since most GRO traffic
> is not trying to kill hosts ;)

Right. I'll send a v2 with this later.


Thanks,

-- 
Sabrina

^ permalink raw reply

* Re: Accelerated receive flow steering (aRFS) for UDP
From: Rick Jones @ 2016-10-10 16:23 UTC (permalink / raw)
  To: Eric Dumazet, Chopra, Manish
  Cc: netdev@vger.kernel.org, maorg@mellanox.com, tom@herbertland.com
In-Reply-To: <ed0f723c-5e66-6d23-086d-71744570700a@hpe.com>

On 10/10/2016 09:08 AM, Rick Jones wrote:
> On 10/09/2016 03:33 PM, Eric Dumazet wrote:
>> OK, I am adding/CC Rick Jones, netperf author, since it seems a netperf
>> bug, not a kernel one.
>>
>> I believe I already mentioned fact that "UDP_STREAM -- -N" was not doing
>> a connect() on the receiver side.
>
> I can confirm that the receive side of the netperf omni path isn't
> trying to connect UDP datagrams.  I will see what I can put together.

I've put something together and pushed it to the netperf top of trunk. 
It seems to have been successful on a quick loopback UDP_STREAM test.

happy benchmarking,

rick jones

^ permalink raw reply

* Re: slab corruption with current -git (was Re: [git pull] vfs pile 1 (splice))
From: Linus Torvalds @ 2016-10-10 16:28 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Florian Westphal, Al Viro, Andrew Morton, Jens Axboe,
	Ted Ts'o, Christoph Lameter, David Miller, Pablo Neira Ayuso,
	Linux Kernel Mailing List, linux-fsdevel, Network Development,
	NetFilter
In-Reply-To: <f7ta8ec71qq.fsf@redhat.com>

On Mon, Oct 10, 2016 at 6:49 AM, Aaron Conole <aconole@redhat.com> wrote:
>
> Okay, I'm looking it over.  Sorry for the mess.

So as I already answered to Dave, I'm not actually sure that this was
the buggy code, or that my patch would make any difference at all.

I never got a good reproducer for the bug: I spent much of the weekend
rebooting, because it seems to happen only just after a reboot, as I
log in and start my usual thing.

I initially blamed some off filesystem or block layer issue ("Oh, it
only happens with a cold cache"), partly because the initial
non-poisoned slub oopses happened in filesystem code.

But I now think it's netfilter, and I *think* that what triggers it is
something like the bluetooth subsystem giving up or something. What I
do when I log into a new session tends to be to go to the kernel
subdirectory in one or two terminals, and fire up chrome to read
email. And the problem either happened within half a minute of me
doing that, or it never happens at all.

Which is why I ended up rebooting a *lot*. Just running the kernel
never triggered it.

(It took me some time to figure that out, which is basically why I did
almost no pull requests the whole weekend)

The journal entries for that invalid kernel access is somewhat suggestive:

  Oct 09 13:24:03 i7 dbus-daemon[1030]: [system] Failed to activate
service 'org.bluez': timed out

  Oct 09 13:24:09 i7 audit[1]: SERVICE_STOP pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
msg='unit=systemd-hostnamed comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'

  Oct 09 13:24:09 i7 kernel: general protection fault: 0000 [#1] SMP

so it happened just as *some* network setup thing was finishing off (I
don't think it was systemd-hostnamed itself that necessarily matters,
but clearly something was finishing up as the netfilter problem
occurred.

> I'll review it, and test it.  Can you tell me what steps you took to
> reproduce the oops?

See above: I can't actually really "reproduce" it. It's probably
highly timing-dependent, and it is not unlikely that it's also very
much about specific setup. I'm running plain Fedora 24, I boot up, log
in, start two or three terminals, fire up chrome, and ...

So far I've seen the problem maybe 5-6 times, but a couple of those
were just silent hangs (I may have rebooted too quickly for things to
hit the disk, or the oops may just have killed the machine too hard).
Two I got the oops inside slub code, and I only have one successful
slub poisoning oops from netfilter.

(Part of the reason I only have one is that once I got that, I stopped
rebooting, and instead started looking at the netfilter code and
started to do some merge window pulls again because I felt that this
is *probably* the core reason, and I cant' afford to not do pulls
during the merge window for _too_ long).

              Linus

^ permalink raw reply

* [PATCH iproute2 1/1] tc filters: add support to get individual filters by handle
From: Jamal Hadi Salim @ 2016-10-10 16:45 UTC (permalink / raw)
  To: stephen; +Cc: netdev, edumazet, mrv, Jamal Hadi Salim

From: Jamal Hadi Salim <jhs@mojatatu.com>

sudo $TC filter add dev $ETH parent ffff: prio 2 protocol ip \
u32 match u32 0 0 flowid 1:1 \
action ok
sudo $TC filter add dev $ETH parent ffff: prio 1 protocol ip \
u32 match ip protocol 1 0xff flowid 1:10 \
action ok

now dump to see all rules..
$TC -s filter ls dev $ETH parent ffff: protocol ip
 ....
filter pref 1 u32
filter pref 1 u32 fh 801: ht divisor 1
filter pref 1 u32 fh 801::800 order 2048 key ht 801 bkt 0 flowid 1:10  (rule hit 0 success 0)
  match 00010000/00ff0000 at 8 (success 0 )
        action order 1: gact action drop
         random type none pass val 0
         index 6 ref 1 bind 1 installed 4 sec used 4 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

filter pref 2 u32
filter pref 2 u32 fh 800: ht divisor 1
filter pref 2 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:1  (rule hit 336 success 336)
  match 00000000/00000000 at 0 (success 336 )
        action order 1: gact action pass
         random type none pass val 0
         index 5 ref 1 bind 1 installed 38 sec used 4 sec
        Action statistics:
        Sent 24864 bytes 336 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0
 ....

..get filter 801::800
$TC -s filter get dev $ETH parent ffff: protocol ip \
handle 801:0:800 prio 2  u32

 ....
filter parent ffff: protocol ip pref 1 u32 fh 801::800 order 2048 key ht 801 bkt 0 flowid 1:10  (rule hit 260 success 130)
  match 00010000/00ff0000 at 8 (success 130 )
        action order 1: gact action drop
         random type none pass val 0
         index 6 ref 1 bind 1 installed 348 sec used 0 sec
        Action statistics:
        Sent 11440 bytes 130 pkt (dropped 130, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0
 ....

..get other one
$TC -s filter get dev $ETH parent ffff: protocol ip \
handle 800:0:800 prio 2  u32

....
filter parent ffff: protocol ip pref 2 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:1  (rule hit 514 success 514)
  match 00000000/00000000 at 0 (success 514 )
        action order 1: gact action pass
         random type none pass val 0
         index 5 ref 1 bind 1 installed 506 sec used 4 sec
        Action statistics:
        Sent 35544 bytes 514 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0
....

..try something that doesnt exist
$TC -s filter get dev $ETH parent ffff: protocol ip  handle 800:0:803 prio 2  u32

.....
RTNETLINK answers: No such file or directory
We have an error talking to the kernel
.....

Note, added NLM_F_ECHO is for backward compatibility. old kernels never
before Eric's patch will not respond without it and newer kernels (after Erics patch)
will ignore it.
In old kernels there is a side effect:
In addition to a response to the GET you will receive an event (if you do tc mon).
But this is still better than what it was before (not working at all).

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tc/tc_filter.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 175 insertions(+), 10 deletions(-)

diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index 7e78e13..2413cef 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -29,6 +29,7 @@
 static void usage(void)
 {
 	fprintf(stderr, "Usage: tc filter [ add | del | change | replace | show ] dev STRING\n");
+	fprintf(stderr, "Usage: tc filter get dev STRING parent CLASSID protocol PROTO handle FILTERID pref PRIO FILTER_TYPE \n");
 	fprintf(stderr, "       [ pref PRIO ] protocol PROTO\n");
 	fprintf(stderr, "       [ estimator INTERVAL TIME_CONSTANT ]\n");
 	fprintf(stderr, "       [ root | ingress | egress | parent CLASSID ]\n");
@@ -186,9 +187,7 @@ static __u32 filter_prio;
 static __u32 filter_protocol;
 __u16 f_proto;
 
-int print_filter(const struct sockaddr_nl *who,
-			struct nlmsghdr *n,
-			void *arg)
+int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 {
 	FILE *fp = (FILE *)arg;
 	struct tcmsg *t = NLMSG_DATA(n);
@@ -197,8 +196,10 @@ int print_filter(const struct sockaddr_nl *who,
 	struct filter_util *q;
 	char abuf[256];
 
-	if (n->nlmsg_type != RTM_NEWTFILTER && n->nlmsg_type != RTM_DELTFILTER) {
-		fprintf(stderr, "Not a filter\n");
+	if (n->nlmsg_type != RTM_NEWTFILTER &&
+	    n->nlmsg_type != RTM_GETTFILTER &&
+	    n->nlmsg_type != RTM_DELTFILTER) {
+		fprintf(stderr, "Not a filter(cmd %d)\n", n->nlmsg_type);
 		return 0;
 	}
 	len -= NLMSG_LENGTH(sizeof(*t));
@@ -269,6 +270,169 @@ int print_filter(const struct sockaddr_nl *who,
 	return 0;
 }
 
+static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
+{
+	struct {
+		struct nlmsghdr	n;
+		struct tcmsg		t;
+		char			buf[MAX_MSG];
+	} req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
+		/* NLM_F_ECHO is for backward compatibility. old kernels never
+		 * respond without it and newer kernels will ignore it.
+		 * In old kernels there is a side effect:
+		 * In addition to a response to the GET you will receive an
+		 * event (if you do tc mon).
+		 */
+		.n.nlmsg_flags = NLM_F_REQUEST | NLM_F_ECHO | flags,
+		.n.nlmsg_type = cmd,
+		.t.tcm_parent = TC_H_UNSPEC,
+		.t.tcm_family = AF_UNSPEC,
+	};
+	struct filter_util *q = NULL;
+	__u32 prio = 0;
+	__u32 protocol = 0;
+	int protocol_set = 0;
+	__u32 parent_handle = 0;
+	char *fhandle = NULL;
+	char  d[16] = {};
+	char  k[16] = {};
+
+	while (argc > 0) {
+		if (strcmp(*argv, "dev") == 0) {
+			NEXT_ARG();
+			if (d[0])
+				duparg("dev", *argv);
+			strncpy(d, *argv, sizeof(d)-1);
+		} else if (strcmp(*argv, "root") == 0) {
+			if (req.t.tcm_parent) {
+				fprintf(stderr, "Error: \"root\" is duplicate parent ID\n");
+				return -1;
+			}
+			req.t.tcm_parent = TC_H_ROOT;
+		} else if (strcmp(*argv, "ingress") == 0) {
+			if (req.t.tcm_parent) {
+				fprintf(stderr, "Error: \"ingress\" is duplicate parent ID\n");
+				return -1;
+			}
+			req.t.tcm_parent = TC_H_MAKE(TC_H_CLSACT,
+						     TC_H_MIN_INGRESS);
+		} else if (strcmp(*argv, "egress") == 0) {
+			if (req.t.tcm_parent) {
+				fprintf(stderr, "Error: \"egress\" is duplicate parent ID\n");
+				return -1;
+			}
+			req.t.tcm_parent = TC_H_MAKE(TC_H_CLSACT,
+						     TC_H_MIN_EGRESS);
+		} else if (strcmp(*argv, "parent") == 0) {
+
+			NEXT_ARG();
+			if (req.t.tcm_parent)
+				duparg("parent", *argv);
+			if (get_tc_classid(&parent_handle, *argv))
+				invarg("Invalid parent ID", *argv);
+			req.t.tcm_parent = parent_handle;
+		} else if (strcmp(*argv, "handle") == 0) {
+			NEXT_ARG();
+			if (fhandle)
+				duparg("handle", *argv);
+			fhandle = *argv;
+		} else if (matches(*argv, "preference") == 0 ||
+			   matches(*argv, "priority") == 0) {
+			NEXT_ARG();
+			if (prio)
+				duparg("priority", *argv);
+			if (get_u32(&prio, *argv, 0) || prio > 0xFFFF)
+				invarg("invalid priority value", *argv);
+		} else if (matches(*argv, "protocol") == 0) {
+			__u16 id;
+
+			NEXT_ARG();
+			if (protocol_set)
+				duparg("protocol", *argv);
+			if (ll_proto_a2n(&id, *argv))
+				invarg("invalid protocol", *argv);
+			protocol = id;
+			protocol_set = 1;
+		} else if (matches(*argv, "help") == 0) {
+			usage();
+			return 0;
+		} else {
+			strncpy(k, *argv, sizeof(k)-1);
+
+			q = get_filter_kind(k);
+			argc--; argv++;
+			break;
+		}
+
+		argc--; argv++;
+	}
+
+	if (!protocol_set) {
+		fprintf(stderr, "Must specify filter protocol\n");
+		return -1;
+	}
+
+	if (!prio) {
+		fprintf(stderr, "Must specify filter priority\n");
+		return -1;
+	}
+
+	req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+
+	if (req.t.tcm_parent == TC_H_UNSPEC) {
+		fprintf(stderr, "Must specify filter parent\n");
+		return -1;
+	}
+
+	if (k[0])
+		addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
+	else {
+		fprintf(stderr, "Must specify filter type\n");
+		return -1;
+	}
+
+	if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
+		return 1;
+
+
+	if (!fhandle) {
+		fprintf(stderr, "Must specify filter \"handle\"\n");
+		return -1;
+	}
+
+	if (argc) {
+		if (matches(*argv, "help") == 0)
+			usage();
+		fprintf(stderr, "Garbage instead of arguments \"%s ...\". Try \"tc filter help\".\n",
+			*argv);
+		return -1;
+	}
+
+	if (d[0])  {
+		ll_init_map(&rth);
+
+		req.t.tcm_ifindex = ll_name_to_index(d);
+		if (req.t.tcm_ifindex  == 0) {
+			fprintf(stderr, "Cannot find device \"%s\"\n", d);
+			return 1;
+		}
+		filter_ifindex = req.t.tcm_ifindex;
+	} else {
+		fprintf(stderr, "Must specify netdevice \"dev\"\n");
+		return -1;
+	}
+
+	if (rtnl_talk(&rth, &req.n, &req.n, MAX_MSG) < 0) {
+		fprintf(stderr, "We have an error talking to the kernel\n");
+		return 2;
+	}
+
+	print_filter(NULL, &req.n, (void *)stdout);
+
+	return 0;
+}
+
 static int tc_filter_list(int argc, char **argv)
 {
 	struct tcmsg t = { .tcm_family = AF_UNSPEC };
@@ -377,17 +541,17 @@ int do_filter(int argc, char **argv)
 	if (argc < 1)
 		return tc_filter_list(0, NULL);
 	if (matches(*argv, "add") == 0)
-		return tc_filter_modify(RTM_NEWTFILTER, NLM_F_EXCL|NLM_F_CREATE, argc-1, argv+1);
+		return tc_filter_modify(RTM_NEWTFILTER, NLM_F_EXCL|NLM_F_CREATE,
+					argc-1, argv+1);
 	if (matches(*argv, "change") == 0)
 		return tc_filter_modify(RTM_NEWTFILTER, 0, argc-1, argv+1);
 	if (matches(*argv, "replace") == 0)
-		return tc_filter_modify(RTM_NEWTFILTER, NLM_F_CREATE, argc-1, argv+1);
+		return tc_filter_modify(RTM_NEWTFILTER, NLM_F_CREATE, argc-1,
+					argv+1);
 	if (matches(*argv, "delete") == 0)
 		return tc_filter_modify(RTM_DELTFILTER, 0,  argc-1, argv+1);
-#if 0
 	if (matches(*argv, "get") == 0)
 		return tc_filter_get(RTM_GETTFILTER, 0,  argc-1, argv+1);
-#endif
 	if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
 	    || matches(*argv, "lst") == 0)
 		return tc_filter_list(argc-1, argv+1);
@@ -395,6 +559,7 @@ int do_filter(int argc, char **argv)
 		usage();
 		return 0;
 	}
-	fprintf(stderr, "Command \"%s\" is unknown, try \"tc filter help\".\n", *argv);
+	fprintf(stderr, "Command \"%s\" is unknown, try \"tc filter help\".\n",
+		*argv);
 	return -1;
 }
-- 
1.9.1

^ permalink raw reply related

* Re: igb driver can cause cache invalidation of non-owned memory?
From: Nikita Yushchenko @ 2016-10-10 17:00 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: David Miller, Jeff Kirsher, intel-wired-lan, Netdev,
	linux-kernel@vger.kernel.org, cphealy
In-Reply-To: <CAKgT0Uc2nL1aPoryhNxdbf3+TQO+fOvAZMZDe+=9NaqnHCZPyw@mail.gmail.com>

Hi Alexander

Thanks for your explanation.

> The main reason why this isn't a concern for the igb driver is because
> we currently pass the page up as read-only.  We don't allow the stack
> to write into the page by keeping the page count greater than 1 which
> means that the page is shared.  It isn't until we unmap the page that
> the page count is allowed to drop to 1 indicating that it is writable.

Doesn't that mean that sync_to_device() in igb_reuse_rx_page() can be
avoided? If page is read only for entire world, then it can't be dirty
in cache and thus device can safely write to it without preparation step.

Nikita


P.S.

We are observing strange performance anomaly with igb on imx6q board.

Test is - simple iperf UDP receive. Other host runs
  iperf -c X.X.X.X -u -b xxxM -t 300 -i 3

Imx6q board can run iperf -s -u, or it can run nothing - result is the same.

While generated traffic (controlled via xxx) is slow, softirq thread on
imx6 board takes near-zero cpu time.  With increasing xxx, it still is
near zero - up to some moment about 700 Mbps. At this moment softirqd
cpu usage suddenly jumps to almost 100%.  Without anything in between:
it is near-zero with slightly smaller traffic, and it is immediately
>99% with slightly larger traffic.

Profiling this situation (>99% in softirqd) with perf gives up to 50%
hits inside cache invalidation loops. That's why originally we thought
cache invalidation is slow. But having the above dependency between
traffic and softirq cpu usage (where napi code runs) can't be explained
with slow cache invalidation.

Also there are additional factors:
- if UDP traffic is dropped - via iptables, or via forcing error paths
at different points of network stack - softirqd cpu usage drops back to
near-zero - although it still does all the same cache invalidations,
- I tried to modify igb driver to disallow page reuse (made
igb_add_rx_frag() always returning false). Result was - "border traffic"
where softirq cpu usage goes from zero to 100% changed from ~700 Mbps to
~400 Mbps.

Any ideas what can happen there, and how to debug it?

TIA

^ permalink raw reply

* Re: Accelerated receive flow steering (aRFS) for UDP
From: Eric Dumazet @ 2016-10-10 17:08 UTC (permalink / raw)
  To: Rick Jones
  Cc: Chopra, Manish, netdev@vger.kernel.org, maorg@mellanox.com,
	tom@herbertland.com
In-Reply-To: <844f9891-3b30-e5fa-720e-d20e4ea5d751@hpe.com>

On Mon, 2016-10-10 at 09:23 -0700, Rick Jones wrote:
> On 10/10/2016 09:08 AM, Rick Jones wrote:
> > On 10/09/2016 03:33 PM, Eric Dumazet wrote:
> >> OK, I am adding/CC Rick Jones, netperf author, since it seems a netperf
> >> bug, not a kernel one.
> >>
> >> I believe I already mentioned fact that "UDP_STREAM -- -N" was not doing
> >> a connect() on the receiver side.
> >
> > I can confirm that the receive side of the netperf omni path isn't
> > trying to connect UDP datagrams.  I will see what I can put together.
> 
> I've put something together and pushed it to the netperf top of trunk. 
> It seems to have been successful on a quick loopback UDP_STREAM test.

Indeed, it looks better, thanks !

^ permalink raw reply

* [PATCH v6 2/4] mac80211: filter multicast data packets on AP / AP_VLAN
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
  To: johannes-cdvu00un1VgdHxzADdlk8Q
  Cc: Michael Braun, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	projekt-wlan-3kN+8DYepx7zMJDuovMtMLNAH6kLmebB,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476119543-24509-1-git-send-email-michael-dev-1SGGS//iJ+Y38rf8aCqVIw@public.gmane.org>

This patch adds filtering for multicast data packets on AP_VLAN interfaces
that have no authorized station connected and changes filtering on AP
interfaces to not count stations assigned to AP_VLAN interfaces.

This saves airtime and avoids waking up other stations currently authorized
in this BSS. When using WPA, the packets dropped could not be decrypted by
any station.

The behaviour when there are no AP_VLAN interfaces is left unchanged.
When there are AP_VLAN interfaces, this patch
1. adds filtering multicast data packets sent on AP_VLAN interfaces that
   have no authorized station connected.
   No filtering happens on 4addr AP_VLAN interfaces.
2. makes filtering of multicast data packets sent on AP interfaces depend
   on the number of authorized stations in this bss not assigned to an
   AP_VLAN interface.

Therefore, a new num_mcast_sta counter is added for AP_VLAN interfaces.
The existing one for AP interfaces is altered to not track stations
assigned to an AP_VLAN interface.

The new counter is exposed in debugfs.

Signed-off-by: Michael Braun <michael-dev-1SGGS//iJ+Y38rf8aCqVIw@public.gmane.org>

--
v4:
 - update description
v3:
 - reuse existing num_mcast_sta
v2:
 - use separate function to inc/dec mcast_sta counters
 - do not filter in 4addr mode
 - change description
 - change filtering on AP interface (do not count AP_VLAN sta)
 - use new counters regardless of 4addr or not
 - simplify cfg.c:change_station
 - remove no-op change in __cleanup_single_sta
---
 net/mac80211/cfg.c            | 20 ++++++--------------
 net/mac80211/debugfs_netdev.c | 11 +++++++++++
 net/mac80211/ieee80211_i.h    | 33 +++++++++++++++++++++++++++++++++
 net/mac80211/rx.c             |  5 +++--
 net/mac80211/sta_info.c       | 10 ++--------
 net/mac80211/tx.c             |  5 ++---
 6 files changed, 57 insertions(+), 27 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 24133f5..1edb017 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1357,9 +1357,6 @@ static int ieee80211_change_station(struct wiphy *wiphy,
 		goto out_err;
 
 	if (params->vlan && params->vlan != sta->sdata->dev) {
-		bool prev_4addr = false;
-		bool new_4addr = false;
-
 		vlansdata = IEEE80211_DEV_TO_SUB_IF(params->vlan);
 
 		if (params->vlan->ieee80211_ptr->use_4addr) {
@@ -1369,26 +1366,21 @@ static int ieee80211_change_station(struct wiphy *wiphy,
 			}
 
 			rcu_assign_pointer(vlansdata->u.vlan.sta, sta);
-			new_4addr = true;
 			__ieee80211_check_fast_rx_iface(vlansdata);
 		}
 
 		if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-		    sta->sdata->u.vlan.sta) {
+		    sta->sdata->u.vlan.sta)
 			RCU_INIT_POINTER(sta->sdata->u.vlan.sta, NULL);
-			prev_4addr = true;
-		}
+
+		if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+			ieee80211_vif_dec_num_mcast(sta->sdata);
 
 		sta->sdata = vlansdata;
 		ieee80211_check_fast_xmit(sta);
 
-		if (sta->sta_state == IEEE80211_STA_AUTHORIZED &&
-		    prev_4addr != new_4addr) {
-			if (new_4addr)
-				atomic_dec(&sta->sdata->bss->num_mcast_sta);
-			else
-				atomic_inc(&sta->sdata->bss->num_mcast_sta);
-		}
+		if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+			ieee80211_vif_inc_num_mcast(sta->sdata);
 
 		ieee80211_send_layer2_update(sta);
 	}
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index a5ba739..ed7bff4 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -477,6 +477,7 @@ IEEE80211_IF_FILE_RW(tdls_wider_bw);
 IEEE80211_IF_FILE(num_mcast_sta, u.ap.num_mcast_sta, ATOMIC);
 IEEE80211_IF_FILE(num_sta_ps, u.ap.ps.num_sta_ps, ATOMIC);
 IEEE80211_IF_FILE(dtim_count, u.ap.ps.dtim_count, DEC);
+IEEE80211_IF_FILE(num_mcast_sta_vlan, u.vlan.num_mcast_sta, ATOMIC);
 
 static ssize_t ieee80211_if_fmt_num_buffered_multicast(
 	const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -643,6 +644,13 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
 }
 
+static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
+{
+	/* add num_mcast_sta_vlan using name num_mcast_sta */
+	debugfs_create_file("num_mcast_sta", 0400, sdata->vif.debugfs_dir,
+			    sdata, &num_mcast_sta_vlan_ops);
+}
+
 static void add_ibss_files(struct ieee80211_sub_if_data *sdata)
 {
 	DEBUGFS_ADD_MODE(tsf, 0600);
@@ -746,6 +754,9 @@ static void add_files(struct ieee80211_sub_if_data *sdata)
 	case NL80211_IFTYPE_AP:
 		add_ap_files(sdata);
 		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		add_vlan_files(sdata);
+		break;
 	case NL80211_IFTYPE_WDS:
 		add_wds_files(sdata);
 		break;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f56d342..70c0963 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -305,6 +305,7 @@ struct ieee80211_if_vlan {
 
 	/* used for all tx if the VLAN is configured to 4-addr mode */
 	struct sta_info __rcu *sta;
+	atomic_t num_mcast_sta; /* number of stations receiving multicast */
 };
 
 struct mesh_stats {
@@ -1496,6 +1497,38 @@ ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
 	return false;
 }
 
+static inline void
+ieee80211_vif_inc_num_mcast(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.type == NL80211_IFTYPE_AP)
+		atomic_inc(&sdata->u.ap.num_mcast_sta);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+		atomic_inc(&sdata->u.vlan.num_mcast_sta);
+}
+
+static inline void
+ieee80211_vif_dec_num_mcast(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.type == NL80211_IFTYPE_AP)
+		atomic_dec(&sdata->u.ap.num_mcast_sta);
+	else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+		atomic_dec(&sdata->u.vlan.num_mcast_sta);
+}
+
+/* This function returns the number of multicast stations connected to this
+ * interface. It returns -1 if that number is not tracked, that is for netdevs
+ * not in AP or AP_VLAN mode or when using 4addr.
+ */
+static inline int
+ieee80211_vif_get_num_mcast_if(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.type == NL80211_IFTYPE_AP)
+		return atomic_read(&sdata->u.ap.num_mcast_sta);
+	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN && !sdata->u.vlan.sta)
+		return atomic_read(&sdata->u.vlan.num_mcast_sta);
+	return -1;
+}
+
 u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
 				     struct ieee80211_rx_status *status,
 				     unsigned int mpdu_len,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 9617f93..252d922 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2160,7 +2160,8 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 	     sdata->vif.type == NL80211_IFTYPE_AP_VLAN) &&
 	    !(sdata->flags & IEEE80211_SDATA_DONT_BRIDGE_PACKETS) &&
 	    (sdata->vif.type != NL80211_IFTYPE_AP_VLAN || !sdata->u.vlan.sta)) {
-		if (is_multicast_ether_addr(ehdr->h_dest)) {
+		if (is_multicast_ether_addr(ehdr->h_dest) &&
+		    ieee80211_vif_get_num_mcast_if(sdata) != 0) {
 			/*
 			 * send multicast frames both to higher layers in
 			 * local net stack and back to the wireless medium
@@ -2169,7 +2170,7 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 			if (!xmit_skb)
 				net_info_ratelimited("%s: failed to clone multicast frame\n",
 						    dev->name);
-		} else {
+		} else if (!is_multicast_ether_addr(ehdr->h_dest)) {
 			dsta = sta_info_get(sdata, skb->data);
 			if (dsta) {
 				/*
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 76b737d..216ef65 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1882,10 +1882,7 @@ int sta_info_move_state(struct sta_info *sta,
 			if (!sta->sta.support_p2p_ps)
 				ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
 		} else if (sta->sta_state == IEEE80211_STA_AUTHORIZED) {
-			if (sta->sdata->vif.type == NL80211_IFTYPE_AP ||
-			    (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-			     !sta->sdata->u.vlan.sta))
-				atomic_dec(&sta->sdata->bss->num_mcast_sta);
+			ieee80211_vif_dec_num_mcast(sta->sdata);
 			clear_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
 			ieee80211_clear_fast_xmit(sta);
 			ieee80211_clear_fast_rx(sta);
@@ -1893,10 +1890,7 @@ int sta_info_move_state(struct sta_info *sta,
 		break;
 	case IEEE80211_STA_AUTHORIZED:
 		if (sta->sta_state == IEEE80211_STA_ASSOC) {
-			if (sta->sdata->vif.type == NL80211_IFTYPE_AP ||
-			    (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-			     !sta->sdata->u.vlan.sta))
-				atomic_inc(&sta->sdata->bss->num_mcast_sta);
+			ieee80211_vif_inc_num_mcast(sta->sdata);
 			set_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
 			ieee80211_check_fast_xmit(sta);
 			ieee80211_check_fast_rx(sta);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 5023966..c3ce86e 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -331,9 +331,8 @@ ieee80211_tx_h_check_assoc(struct ieee80211_tx_data *tx)
 			I802_DEBUG_INC(tx->local->tx_handlers_drop_not_assoc);
 			return TX_DROP;
 		}
-	} else if (unlikely(tx->sdata->vif.type == NL80211_IFTYPE_AP &&
-			    ieee80211_is_data(hdr->frame_control) &&
-			    !atomic_read(&tx->sdata->u.ap.num_mcast_sta))) {
+	} else if (unlikely(ieee80211_is_data(hdr->frame_control) &&
+			    ieee80211_vif_get_num_mcast_if(tx->sdata) == 0)) {
 		/*
 		 * No associated STAs - no need to send multicast
 		 * frames.
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 1/4] mac80211: remove unnecessary num_mcast_sta user
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
  To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan, netdev

Checking for num_mcast_sta in __ieee80211_request_smps_ap() is unnecessary,
as sta list will be empty in this case anyway, so list_for_each_entry(sta,
...) will exit immediately.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
 net/mac80211/cfg.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 543b1d4..24133f5 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2313,13 +2313,6 @@ int __ieee80211_request_smps_ap(struct ieee80211_sub_if_data *sdata,
 	    smps_mode == IEEE80211_SMPS_AUTOMATIC)
 		return 0;
 
-	 /* If no associated stations, there's no need to do anything */
-	if (!atomic_read(&sdata->u.ap.num_mcast_sta)) {
-		sdata->smps_mode = smps_mode;
-		ieee80211_queue_work(&sdata->local->hw, &sdata->recalc_smps);
-		return 0;
-	}
-
 	ht_dbg(sdata,
 	       "SMPS %d requested in AP mode, sending Action frame to %d stations\n",
 	       smps_mode, atomic_read(&sdata->u.ap.num_mcast_sta));
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 4/4] mac80211: multicast to unicast conversion
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
  To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan, netdev
In-Reply-To: <1476119543-24509-1-git-send-email-michael-dev@fami-braun.de>

This patch adds support for sending multicast data packets with ARP, IPv4
and IPv6 payload (possible 802.1q tagged) as 802.11 unicast frames to all
stations.

IEEE 802.11 multicast has well known issues, among them:
 1. packets are not acked and hence not retransmitted, resulting in
    decreased reliablity
 2. packets are send at low rate, increasing time required on air

When used with AP_VLAN, there is another disadvantage:
 3. all stations in the BSS are woken up, regardsless of their AP_VLAN
    assignment.

By doing multicast to unicast conversion, all three issus are solved.

IEEE802.11-2012 proposes directed multicast service (DMS) using A-MSDU
frames and a station initiated control protocol. It has the advantage that
the station can recover the destination multicast mac address, but it is
not backward compatible with non QOS stations and does not enable the
administrator of a BSS to force this mode of operation within a BSS.
Additionally, it would require both the ap and the station to implement
the control protocol, which is optional on both ends. Furthermore, I've
seen a few mobile phone stations locally that indicate qos support but
won't complete DHCP if their broadcasts are encapsulated as A-MSDU. Though
they work fine with this series approach.

This patch therefore does not opt to implement DMS but instead just
replicates the packet and changes the destination address. As this works
fine with ARP, IPv4 and IPv6, it is limited to these protocols and normal
802.11 multicast frames are send out for all other payload protocols.

There is a runtime toggle to enable multicast conversion in a per-bss
fashion.

When there is only a single station assigned to the AP_VLAN interface, no
packet replication will occur. 4addr mode of operation is unchanged.

This change opts for iterating all BSS stations for finding the stations
assigned to this AP/AP_VLAN interface, as there currently is no per
AP_VLAN list to iterate and multicast packets are expected to be few.
If needed, such a list could be added later.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>

--
v5:
  - rename bss->unicast to bss->multicast_to_unicast
  - access sdata->bss only after checking iftype
v4:
  - rename MULTICAST_TO_UNICAST to MULTICAST_TO_UNICAST
v3: fix compile error for trace.h
v2: add nl80211 toggle
    rename tx_dnat to change_da
    change int to bool unicast
---
 net/mac80211/cfg.c            |  15 +++++++
 net/mac80211/debugfs_netdev.c |   3 ++
 net/mac80211/ieee80211_i.h    |   1 +
 net/mac80211/tx.c             | 101 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1edb017..1db4c3e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2242,6 +2242,20 @@ static int ieee80211_set_wds_peer(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static int ieee80211_set_multicast_to_unicast(struct wiphy *wiphy,
+					      struct net_device *dev,
+					      const bool enabled)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+	if (sdata->vif.type != NL80211_IFTYPE_AP)
+		return -1;
+
+	sdata->u.ap.multicast_to_unicast = enabled;
+
+	return 0;
+}
+
 static void ieee80211_rfkill_poll(struct wiphy *wiphy)
 {
 	struct ieee80211_local *local = wiphy_priv(wiphy);
@@ -3400,6 +3414,7 @@ const struct cfg80211_ops mac80211_config_ops = {
 	.set_tx_power = ieee80211_set_tx_power,
 	.get_tx_power = ieee80211_get_tx_power,
 	.set_wds_peer = ieee80211_set_wds_peer,
+	.set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
 	.rfkill_poll = ieee80211_rfkill_poll,
 	CFG80211_TESTMODE_CMD(ieee80211_testmode_cmd)
 	CFG80211_TESTMODE_DUMP(ieee80211_testmode_dump)
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index ed7bff4..509c6c3 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -487,6 +487,8 @@ static ssize_t ieee80211_if_fmt_num_buffered_multicast(
 }
 IEEE80211_IF_FILE_R(num_buffered_multicast);
 
+IEEE80211_IF_FILE(multicast_to_unicast, u.ap.multicast_to_unicast, HEX);
+
 /* IBSS attributes */
 static ssize_t ieee80211_if_fmt_tsf(
 	const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -642,6 +644,7 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD(dtim_count);
 	DEBUGFS_ADD(num_buffered_multicast);
 	DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
+	DEBUGFS_ADD_MODE(multicast_to_unicast, 0600);
 }
 
 static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 70c0963..84374ed 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -293,6 +293,7 @@ struct ieee80211_if_ap {
 			 driver_smps_mode; /* smps mode request */
 
 	struct work_struct request_smps_work;
+	bool multicast_to_unicast;
 };
 
 struct ieee80211_if_wds {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index c3ce86e..142201d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/skbuff.h>
+#include <linux/if_vlan.h>
 #include <linux/etherdevice.h>
 #include <linux/bitmap.h>
 #include <linux/rcupdate.h>
@@ -1770,6 +1771,102 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
 }
 EXPORT_SYMBOL(ieee80211_tx_prepare_skb);
 
+/* rewrite destination mac address */
+static int ieee80211_change_da(struct sk_buff *skb, struct sta_info *sta)
+{
+	struct ethhdr *eth;
+	int err;
+
+	err = skb_ensure_writable(skb, ETH_HLEN);
+	if (unlikely(err))
+		return err;
+
+	eth = (void *)skb->data;
+	ether_addr_copy(eth->h_dest, sta->sta.addr);
+
+	return 0;
+}
+
+/* Check if multicast to unicast conversion is needed and do it.
+ * Returns 1 if skb was freed and should not be send out.
+ */
+static int
+ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
+				  struct sk_buff *skb, u32  info_flags)
+{
+	struct ieee80211_local *local = sdata->local;
+	const struct ethhdr *eth = (void *)skb->data;
+	const struct vlan_ethhdr *ethvlan = (void *)skb->data;
+	struct sta_info *sta, *prev = NULL;
+	struct sk_buff *cloned_skb;
+	u16 ethertype;
+
+	/* check if this is a multicast frame */
+	if (!is_multicast_ether_addr(eth->h_dest))
+		return 0;
+
+	/* multicast to unicast conversion only for AP interfaces */
+	switch (sdata->vif.type) {
+	case NL80211_IFTYPE_AP_VLAN:
+		sta = rcu_dereference(sdata->u.vlan.sta);
+		if (sta) /* 4addr */
+			return 0;
+		/* fall through */
+	case NL80211_IFTYPE_AP:
+		/* check runtime toggle for this bss */
+		if (!sdata->bss->multicast_to_unicast)
+			return 0;
+		break;
+	default:
+		return 0;
+	}
+
+	/* info_flags would not get preserved, used only by TLDS */
+	if (info_flags)
+		return 0;
+
+	/* multicast to unicast conversion only for some payload */
+	ethertype = ntohs(eth->h_proto);
+	if (ethertype == ETH_P_8021Q && skb->len >= VLAN_ETH_HLEN)
+		ethertype = ntohs(ethvlan->h_vlan_encapsulated_proto);
+	switch (ethertype) {
+	case ETH_P_ARP:
+	case ETH_P_IP:
+	case ETH_P_IPV6:
+		break;
+	default:
+		return 0;
+	}
+
+	/* clone packets and update destination mac */
+	list_for_each_entry_rcu(sta, &local->sta_list, list) {
+		if (sdata != sta->sdata)
+			continue;
+		if (unlikely(!ether_addr_equal(eth->h_source, sta->sta.addr)))
+			/* do not send back to source */
+			continue;
+		if (!prev) {
+			prev = sta;
+			continue;
+		}
+		cloned_skb = skb_clone(skb, GFP_ATOMIC);
+		if (unlikely(ieee80211_change_da(cloned_skb, prev))) {
+			dev_kfree_skb(cloned_skb);
+			continue;
+		}
+		__ieee80211_subif_start_xmit(cloned_skb, cloned_skb->dev, 0);
+		prev = sta;
+	}
+
+	if (likely(prev)) {
+		ieee80211_change_da(skb, prev);
+		return 0;
+	}
+
+	/* no STA connected, drop */
+	return 1;
+}
+
 /*
  * Returns false if the frame couldn't be transmitted but was queued instead.
  */
@@ -3353,6 +3450,10 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 
 	rcu_read_lock();
 
+	/* AP multicast to unicast conversion */
+	if (ieee80211_tx_multicast_to_unicast(sdata, skb, info_flags))
+		goto out_free;
+
 	if (ieee80211_lookup_ra_sta(sdata, skb, &sta))
 		goto out_free;
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 3/4] cfg80211: configure multicast to unicast for AP interfaces
From: Michael Braun @ 2016-10-10 17:12 UTC (permalink / raw)
  To: johannes; +Cc: Michael Braun, linux-wireless, projekt-wlan, netdev
In-Reply-To: <1476119543-24509-1-git-send-email-michael-dev@fami-braun.de>

This add a userspace toggle to configure multicast to unicast.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>

--
v6:
 - clarify documentation
 - fix policy for NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED
---
 include/net/cfg80211.h       |  6 ++++++
 include/uapi/linux/nl80211.h | 18 ++++++++++++++++++
 net/wireless/nl80211.c       | 36 ++++++++++++++++++++++++++++++++++++
 net/wireless/rdev-ops.h      | 12 ++++++++++++
 net/wireless/trace.h         | 19 +++++++++++++++++++
 5 files changed, 91 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7ce6223..7b0941d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2460,6 +2460,8 @@ struct cfg80211_qos_map {
  *
  * @set_wds_peer: set the WDS peer for a WDS interface
  *
+ * @set_multicast_to_unicast: configure multicast to unicast conversion for BSS
+ *
  * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting
  *	functions to adjust rfkill hw state
  *
@@ -2722,6 +2724,10 @@ struct cfg80211_ops {
 	int	(*set_wds_peer)(struct wiphy *wiphy, struct net_device *dev,
 				const u8 *addr);
 
+	int	(*set_multicast_to_unicast)(struct wiphy *wiphy,
+					    struct net_device *dev,
+					    const bool enabled);
+
 	void	(*rfkill_poll)(struct wiphy *wiphy);
 
 #ifdef CONFIG_NL80211_TESTMODE
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 2206941..327bbb8 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -599,6 +599,17 @@
  *
  * @NL80211_CMD_SET_WDS_PEER: Set the MAC address of the peer on a WDS interface.
  *
+ * @NL80211_CMD_SET_MULTICAST_TO_UNICAST: Configure if this AP should perform
+ *      multicast to unicast conversion. When enabled, all multicast packets
+ *      with ethertype ARP, IPv4 or IPv6 (possibly within an 802.1q header)
+ *      will be sent out to each station once with the destination (multicast)
+ *      mac address replaced by the stations mac address.
+ *      This can only be toggled per BSS. Configure this on an interface of
+ *      type %NL80211_IFTYPE_AP. It applies to all its vlans interfaces
+ *      (%NL80211_IFTYPE_AP_VLAN), except for those in 4addr (WDS) mode.
+ *      If %NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED is not present with this
+ *      command, the feature is disabled.
+ *
  * @NL80211_CMD_JOIN_MESH: Join a mesh. The mesh ID must be given, and initial
  *	mesh config parameters may be given.
  * @NL80211_CMD_LEAVE_MESH: Leave the mesh network -- no special arguments, the
@@ -1026,6 +1037,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_ABORT_SCAN,
 
+	NL80211_CMD_SET_MULTICAST_TO_UNICAST,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -1867,6 +1880,9 @@ enum nl80211_commands {
  * @NL80211_ATTR_MESH_PEER_AID: Association ID for the mesh peer (u16). This is
  *	used to pull the stored data for mesh peer in power save state.
  *
+ * @NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED: Multicast packets should be
+ *      send out as unicast to all stations.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2261,6 +2277,8 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_MESH_PEER_AID,
 
+	NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f02653a..3684e28 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -409,6 +409,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 		.len = VHT_MUMIMO_GROUPS_DATA_LEN
 	},
 	[NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = { .len = ETH_ALEN },
+	[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
 };
 
 /* policy for the key attributes */
@@ -1538,6 +1539,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 				goto nla_put_failure;
 		}
 		CMD(set_wds_peer, SET_WDS_PEER);
+		CMD(set_multicast_to_unicast, SET_MULTICAST_TO_UNICAST);
 		if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS) {
 			CMD(tdls_mgmt, TDLS_MGMT);
 			CMD(tdls_oper, TDLS_OPER);
@@ -2164,6 +2166,32 @@ static int nl80211_set_wds_peer(struct sk_buff *skb, struct genl_info *info)
 	return rdev_set_wds_peer(rdev, dev, bssid);
 }
 
+static int nl80211_set_multicast_to_unicast(struct sk_buff *skb,
+					    struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	const struct nlattr *nla;
+	bool enabled;
+
+	if (!info->attrs[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED])
+		return -EINVAL;
+
+	if (netif_running(dev))
+		return -EBUSY;
+
+	if (!rdev->ops->set_multicast_to_unicast)
+		return -EOPNOTSUPP;
+
+	if (wdev->iftype != NL80211_IFTYPE_AP)
+		return -EOPNOTSUPP;
+
+	nla = info->attrs[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED];
+	enabled = nla_get_flag(nla);
+	return rdev_set_multicast_to_unicast(rdev, dev, enabled);
+}
+
 static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev;
@@ -11574,6 +11602,14 @@ static const struct genl_ops nl80211_ops[] = {
 				  NL80211_FLAG_NEED_RTNL,
 	},
 	{
+		.cmd = NL80211_CMD_SET_MULTICAST_TO_UNICAST,
+		.doit = nl80211_set_multicast_to_unicast,
+		.policy = nl80211_policy,
+		.flags = GENL_UNS_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_NETDEV |
+				  NL80211_FLAG_NEED_RTNL,
+	},
+	{
 		.cmd = NL80211_CMD_JOIN_MESH,
 		.doit = nl80211_join_mesh,
 		.policy = nl80211_policy,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 85ff30b..7d93c3d 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -562,6 +562,18 @@ static inline int rdev_set_wds_peer(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
+static inline int
+rdev_set_multicast_to_unicast(struct cfg80211_registered_device *rdev,
+			      struct net_device *dev,
+			      const bool enabled)
+{
+	int ret;
+	trace_rdev_set_multicast_to_unicast(&rdev->wiphy, dev, enabled);
+	ret = rdev->ops->set_multicast_to_unicast(&rdev->wiphy, dev, enabled);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
+
 static inline void rdev_rfkill_poll(struct cfg80211_registered_device *rdev)
 {
 	trace_rdev_rfkill_poll(&rdev->wiphy);
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 72b5255..8c9aa57 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2940,6 +2940,25 @@ DEFINE_EVENT(wiphy_wdev_evt, rdev_abort_scan,
 	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
 	TP_ARGS(wiphy, wdev)
 );
+
+TRACE_EVENT(rdev_set_multicast_to_unicast,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
+		 const bool enabled),
+	TP_ARGS(wiphy, netdev, enabled),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		NETDEV_ENTRY
+		__field(bool, enabled)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		NETDEV_ASSIGN;
+		__entry->enabled = enabled;
+	),
+	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", unicast: %s",
+		  WIPHY_PR_ARG, NETDEV_PR_ARG,
+		  BOOL_TO_STR(__entry->enabled))
+);
 #endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.1.4

^ permalink raw reply related

* Re: igb driver can cause cache invalidation of non-owned memory?
From: Alexander Duyck @ 2016-10-10 17:38 UTC (permalink / raw)
  To: Nikita Yushchenko, Eric Dumazet
  Cc: David Miller, Jeff Kirsher, intel-wired-lan, Netdev,
	linux-kernel@vger.kernel.org, cphealy
In-Reply-To: <19aebfc3-5f1a-9206-4493-2255af7269f9@cogentembedded.com>

On Mon, Oct 10, 2016 at 10:00 AM, Nikita Yushchenko
<nikita.yoush@cogentembedded.com> wrote:
> Hi Alexander
>
> Thanks for your explanation.
>
>> The main reason why this isn't a concern for the igb driver is because
>> we currently pass the page up as read-only.  We don't allow the stack
>> to write into the page by keeping the page count greater than 1 which
>> means that the page is shared.  It isn't until we unmap the page that
>> the page count is allowed to drop to 1 indicating that it is writable.
>
> Doesn't that mean that sync_to_device() in igb_reuse_rx_page() can be
> avoided? If page is read only for entire world, then it can't be dirty
> in cache and thus device can safely write to it without preparation step.

For the sake of correctness we were adding the
dma_sync_single_range_for_device.  Since it is an DMA_FROM_DEVICE
mapping calling it should really have no effect for most DMA mapping
interfaces.

Also you may want to try updating to the 4.8 version of the driver.
It reduces the size of the dma_sync_single_range_for_cpu loops by
reducing the sync size down to the size that was DMAed into the
buffer.

> Nikita
>
>
> P.S.
>
> We are observing strange performance anomaly with igb on imx6q board.
>
> Test is - simple iperf UDP receive. Other host runs
>   iperf -c X.X.X.X -u -b xxxM -t 300 -i 3
>
> Imx6q board can run iperf -s -u, or it can run nothing - result is the same.
>
> While generated traffic (controlled via xxx) is slow, softirq thread on
> imx6 board takes near-zero cpu time.  With increasing xxx, it still is
> near zero - up to some moment about 700 Mbps. At this moment softirqd
> cpu usage suddenly jumps to almost 100%.  Without anything in between:
> it is near-zero with slightly smaller traffic, and it is immediately
>>99% with slightly larger traffic.
>
> Profiling this situation (>99% in softirqd) with perf gives up to 50%
> hits inside cache invalidation loops. That's why originally we thought
> cache invalidation is slow. But having the above dependency between
> traffic and softirq cpu usage (where napi code runs) can't be explained
> with slow cache invalidation.
>
> Also there are additional factors:
> - if UDP traffic is dropped - via iptables, or via forcing error paths
> at different points of network stack - softirqd cpu usage drops back to
> near-zero - although it still does all the same cache invalidations,
> - I tried to modify igb driver to disallow page reuse (made
> igb_add_rx_frag() always returning false). Result was - "border traffic"
> where softirq cpu usage goes from zero to 100% changed from ~700 Mbps to
> ~400 Mbps.
>
> Any ideas what can happen there, and how to debug it?
>
> TIA

I'm adding Eric Dumazet as he is more of an expert on all things NAPI
than I am, but it is my understanding that there are known issues in
regards to how the softirq traffic is handled.  Specifically I believe
the 0->100% accounting problem is due to the way this is all tracked.
You may want to try pulling the most recent net-next kernel and
testing that to see if you still see the same behavior as Eric has
recently added a fix that is meant to allow for better sharing between
softirq polling and applications when dealing with stuff like UDP
traffic.

As far as identifying the problem areas your best bet would be to push
the CPU to 100% and then identify the hot spots.

- Alex

^ permalink raw reply

* [RFC] net: phy: smsc: Disable auto-negotiation on startup
From: Kyle Roeschley @ 2016-10-10 17:41 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, linux-kernel, Kyle Roeschley

Because the SMSC PHY completes auto-negotiation before the driver is
ready to handle interrupts, the PHY state machine never realizes that we
have a link. Clear the ANENABLE bit on initialization, which lets
genphy_config_aneg do its thing when that code is hit later.

While this patch does fix the problem we see (no link on boot without
re-plugging the cable), it seems like the generic PHY code should be
able to handle auto-negotiation completing before interrupts are
enabled. Submitted as an RFC in the hopes that someone has an idea as to
how that could be done.

This fix is copied from commit 99f81afc139c ("phy: micrel: Disable auto
negotiation on startup").

Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com>
---
 drivers/net/phy/smsc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index b62c4aa..8de8011 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -62,6 +62,16 @@ static int smsc_phy_config_init(struct phy_device *phydev)
 			return rc;
 	}
 
+	if (phy_interrupt_is_valid(phydev)) {
+		rc = phy_read(phydev, MII_BMCR);
+		if (rc < 0)
+			return rc;
+
+		rc = phy_write(phydev, MII_BMCR, rc & ~BMCR_ANENABLE);
+		if (rc < 0)
+			return rc;
+	}
+
 	return smsc_phy_ack_interrupt(phydev);
 }
 
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH net-next] sctp: reuse sent_count to avoid retransmitted chunks for RTT measurements
From: Marcelo Ricardo Leitner @ 2016-10-10 17:48 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, Vlad Yasevich, daniel, davem
In-Reply-To: <b68194a4735a1ea5a652c13540df5d68e3c4b305.1475897765.git.lucien.xin@gmail.com>

On Sat, Oct 08, 2016 at 11:36:05AM +0800, Xin Long wrote:
> Now sctp uses chunk->resent to record if a chunk is retransmitted, for
> RTT measurements with retransmitted DATA chunks. chunk->sent_count was
> introduced to record how many times one chunk has been sent for prsctp
> RTX policy before. We actually can know if one chunk is retransmitted
> by checking chunk->sent_count is greater than 1.
> 
> This patch is to remove resent from sctp_chunk and reuse sent_count
> to avoid retransmitted chunks for RTT measurements.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/net/sctp/structs.h | 2 +-
>  net/sctp/output.c          | 3 ++-
>  net/sctp/outqueue.c        | 4 +---
>  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 11c3bf2..27a933e 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -641,7 +641,6 @@ struct sctp_chunk {
>  #define SCTP_NEED_FRTX 0x1
>  #define SCTP_DONT_FRTX 0x2
>  	__u16	rtt_in_progress:1,	/* This chunk used for RTT calc? */
> -		resent:1,		/* Has this chunk ever been resent. */
>  		has_tsn:1,		/* Does this chunk have a TSN yet? */
>  		has_ssn:1,		/* Does this chunk have a SSN yet? */
>  		singleton:1,		/* Only chunk in the packet? */
> @@ -656,6 +655,7 @@ struct sctp_chunk {
>  		fast_retransmit:2;	/* Is this chunk fast retransmitted? */
>  };
>  
> +#define sctp_chunk_retransmitted(chunk)	(chunk->sent_count > 1)
>  void sctp_chunk_hold(struct sctp_chunk *);
>  void sctp_chunk_put(struct sctp_chunk *);
>  int sctp_user_addto_chunk(struct sctp_chunk *chunk, int len,
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 2a5c189..84f66d5 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -552,7 +552,8 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
>  				 * for a given destination transport address.
>  				 */
>  
> -				if (!chunk->resent && !tp->rto_pending) {
> +				if (!sctp_chunk_retransmitted(chunk) &&
> +				    !tp->rto_pending) {
>  					chunk->rtt_in_progress = 1;
>  					tp->rto_pending = 1;
>  				}
> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index 5825853..e540826 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -507,8 +507,6 @@ void sctp_retransmit_mark(struct sctp_outq *q,
>  				transport->rto_pending = 0;
>  			}
>  
> -			chunk->resent = 1;
> -
>  			/* Move the chunk to the retransmit queue. The chunks
>  			 * on the retransmit queue are always kept in order.
>  			 */
> @@ -1439,7 +1437,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
>  				 * instance).
>  				 */
>  				if (!tchunk->tsn_gap_acked &&
> -				    !tchunk->resent &&
> +				    !sctp_chunk_retransmitted(tchunk) &&
>  				    tchunk->rtt_in_progress) {
>  					tchunk->rtt_in_progress = 0;
>  					rtt = jiffies - tchunk->sent_at;
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* HSR status queries from user space
From: Murali Karicheri @ 2016-10-10 18:13 UTC (permalink / raw)
  To: arvid.brodin,
	netdev@vger.kernel.org >> open list:TI NETCP ETHERNET DRIVER

Hi Arvid,

I am investigating the HSR implementation in Linux and wondering if
there is a tool that I can use to query the status of the HSR nodes.
Especially data related to HSR_C_GET_NODE_STATUS and HSR_C_GET_NODE_LIST
commands.

I am able to setup hsr interface using two Ethernet slave interfaces on
my platform and I am able to ping between the two boards. I am looking
to dump the status of the nodes. Could you tell me what tool to use?

ip link command is used to create the hsr interface, but can't find
any tool to dump the node list and status.

Thanks

-- 
Murali Karicheri
Linux Kernel, Keystone

^ permalink raw reply

* Re: kernel v4.8: iptables logs are truncated with the 4.8 kernel?
From: Chris Caputo @ 2016-10-10 18:33 UTC (permalink / raw)
  To: Liping Zhang
  Cc: Vishwanath Pai, Pablo Neira Ayuso, Justin Piszcz, linux-kernel,
	Linux Kernel Network Developers
In-Reply-To: <CAML_gOeUOrt=-D3VHS2B3B_Q1F2-DAYK95Oxu4FEvfE43hjb3g@mail.gmail.com>

On Mon, 10 Oct 2016, Liping Zhang wrote:
> 2016-10-10 15:02 GMT+08:00 Chris Caputo <ccaputo@alt.net>:
> >   Program received signal SIGSEGV, Segmentation fault.
> >   0x00007ffff65fd18a in _interp_iphdr (pi=0x617f50, len=0) at ulogd_raw2packet_BASE.c:720
> >
> >   715     static int _interp_iphdr(struct ulogd_pluginstance *pi, uint32_t len)
> >   716     {
> >   717             struct ulogd_key *ret = pi->output.keys;
> >   718             struct iphdr *iph =
> >   719                     ikey_get_ptr(&pi->input.keys[INKEY_RAW_PCKT]);
> >   720             void *nexthdr = (uint32_t *)iph + iph->ihl;
> >
> > I believe 7643507fe8b5bd8ab7522f6a81058cc1209d2585 changed previous
> > behavior by not always copying IP header data to user space.
> >
> > On my machine IPv4 log packets result in a ulogd segfault while IPv6
> > packets do not.  I'm not sure of the cause of the difference.
> >
> > The corresponding userspace commit for the 209d2585 kernel change is:
> >
> >   https://git.netfilter.org/iptables/commit/?id=7070b1f3c88a0c3d4e315c00cca61f05b0fbc882
> >
> > This adds --nflog-size to iptables.  When --nflog-size is used with my
> > iptables NFLOG lines, the ulogd-2.0.5 segfaults cease.
> 
> What numbers did you specify after --nflog-size option?
> --nflog-size 0 or ...? If you want log the whole packet to
> the ulogd, please do not specify this nflog-size option.

Not specifying nflog-size does not appear to log the whole packet...

If "--nflog-size" is unspecified, and the iptables config is left 
unchanged when the kernel is upgraded to 4.8, ulogd-2.0.5 crashes.

If "--nflog-size 0" is used, ulogd-2.0.5 crashes.

If "--nflog-size" is used with size 1 or greater, ulogd-2.0.5 is fine.

> > I'm surprised to see a kernel change cause unexpected userspace segfaults,
> > so further investigation into a kernel fix would seem a good idea.
> 
> According to the original user's manual, nflog-range option was
> designed to be the number of bytes copied to userspace, but
> unfortunately there's a bug from the beginning and it never works,
> i.e. in kernel, it just ignored this option.
> 
> Try to change the current nflog-range option's semantics may
> cause unexpected results(maybe like this ulogd crash) ...
> 
> In order to keep compatibility, Vishwanath introduce a new
> nflog-size option and keep nflog-range unchanged. If you just
> upgrade the kernel, and do not change iptables rules, this
> problem will not happen.

I am reporting that the problem does happen simply with an upgrade to 
kernel 4.8 and no other changes.  When "--nflog-size" is unspecified or 
set to 0, the bug in ulogd-2.0.5 gets triggered.

I agree there is a bug in ulogd-2.0.5 that this kernel change exposed, but 
I am trying to explain that all ulogd users risk this segfault if they 
upgrade to kernel 4.8 and don't either update to a fixed ulogd (possibly 
using your patch below) or an unreleased iptables with iptables config 
changes to implement nflog-size on each NFLOG target.

> So I think this is ulogd's bug, in _interp_iphdr, it try to
> dereference the iphdr pointer before validation check, meanwhile
> this problem does not exist in ipv6 path.  Can you try this patch:
> 
> diff --git a/filter/raw2packet/ulogd_raw2packet_BASE.c
> b/filter/raw2packet/ulogd_raw2packet_BASE.c
> index 8a6180c..fd2665a 100644
> --- a/filter/raw2packet/ulogd_raw2packet_BASE.c
> +++ b/filter/raw2packet/ulogd_raw2packet_BASE.c
> @@ -717,7 +717,7 @@ static int _interp_iphdr(struct ulogd_pluginstance
> *pi, uint32_t len)
>         struct ulogd_key *ret = pi->output.keys;
>         struct iphdr *iph =
>                 ikey_get_ptr(&pi->input.keys[INKEY_RAW_PCKT]);
> -       void *nexthdr = (uint32_t *)iph + iph->ihl;
> +       void *nexthdr;
> 
>         if (len < sizeof(struct iphdr) || len <= (uint32_t)(iph->ihl * 4))
>                 return ULOGD_IRET_OK;
> @@ -734,6 +734,7 @@ static int _interp_iphdr(struct ulogd_pluginstance
> *pi, uint32_t len)
>         okey_set_u16(&ret[KEY_IP_ID], ntohs(iph->id));
>         okey_set_u16(&ret[KEY_IP_FRAGOFF], ntohs(iph->frag_off));
> 
> +       nexthdr = (uint32_t *)iph + iph->ihl;
>         switch (iph->protocol) {
>         case IPPROTO_TCP:
>                 _interp_tcp(pi, nexthdr, len);

I agree this will likely fix ulogd, but this misses the point about the 
new kernel defaulting to a zero size return when it used to return the 
packet.

Thanks,
Chris

^ permalink raw reply

* Redundancy support through PRP?
From: Murali Karicheri @ 2016-10-10 18:34 UTC (permalink / raw)
  To: open list:TI NETCP ETHERNET DRIVER

All,

Wondering if there plan to add PRP driver support, like HSR in Linux? AFAIK, PRP
adds trailor to Ethernet frame and is used for Redundancy management like HSR.
So wondering why this is not supported.

Thanks
-- 
Murali Karicheri
Linux Kernel, Keystone

^ permalink raw reply

* Re: slab corruption with current -git (was Re: [git pull] vfs pile 1 (splice))
From: Linus Torvalds @ 2016-10-10 19:05 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Florian Westphal, Al Viro, Andrew Morton, Jens Axboe,
	Ted Ts'o, Christoph Lameter, David Miller, Pablo Neira Ayuso,
	Linux Kernel Mailing List, linux-fsdevel, Network Development,
	NetFilter
In-Reply-To: <CA+55aFy6pRhvodYG9WWjfxQBFr4ExSk_wxwY=treWYTPK+NTvQ@mail.gmail.com>

On Mon, Oct 10, 2016 at 9:28 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So as I already answered to Dave, I'm not actually sure that this was
> the buggy code, or that my patch would make any difference at all.

My patch does seem to fix things, and in fact the warning about "hook
not found" now triggers.

So I think the bug really was that the singly-linked list handling
code did not correctly handle the case of not finding the entry, and
then freed (incorrectly) the last one that wasn't actually unlinked.

In fact, I get quite a few warnings (56 total) about 30 seconds after
logging in:

[   54.213170] WARNING: CPU: 1 PID: 111 at net/netfilter/core.c:151
nf_unregister_net_hook+0x8e/0x170
... repeat 54 times ...
[   54.445520] WARNING: CPU: 7 PID: 111 at net/netfilter/core.c:151
nf_unregister_net_hook+0x8e/0x170

and looking in the journal, the first one is (again) immediately
preceded by that systemd-hostnamed service stopping:

  Oct 10 11:45:47 i7 audit[1546]: USER_LOGIN
  ...
  Oct 10 11:46:11 i7 audit[1]: SERVICE_STOP pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
msg='unit=fprintd comm="systemd" exe="/usr/lib/systemd/systemd"
hostname=? addr=? terminal=? res=success'
  Oct 10 11:46:13 i7 pulseaudio[1697]: [pulseaudio] bluez5-util.c:
GetManagedObjects() failed: org.freedesktop.DBus.Error.NoReply: Did
not receive a reply. Possible causes include: the remote application
did not send a reply, the message bus security policy blocked the
reply, the reply timeout expir
  Oct 10 11:46:13 i7 dbus-daemon[1003]: [system] Failed to activate
service 'org.bluez': timed out
  Oct 10 11:46:20 i7 audit[1]: SERVICE_STOP pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
msg='unit=systemd-hostnamed comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
  Oct 10 11:46:20 i7 kernel: ------------[ cut here ]------------
  Oct 10 11:46:20 i7 kernel: WARNING: CPU: 1 PID: 111 at
net/netfilter/core.c:151 nf_unregister_net_hook+0x8e/0x170

so I do think it's something to do with some network startup service
thing (perhaps dhcp, perhaps chrome, who knows) as I do my initial
login.

David - I think that also explains what was wrong with the old code.
In the old code, this loop:

        while (hooks_entry && nf_entry_dereference(hooks_entry->next)) {

would exit with "hooks_entry" pointing to the last list entry (because
->next was NULL). Nothing was ever unlinked in the loop itself,
because it never actually found a matching entry, but then after the
loop it would free that last entry because it *thought* that was the
match.

My list rewrite fixes that.

Anyway, I'm assuming it will come to me from the networking tree after
more testing by the maintainers. You can add my

  Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

to the patch, though.

David, if you want me to just commit that thing directly, I can
obviously do so, but I do think somebody should look at

 (a) that I actually got the priority list ordering right on the insertion side
 (b) what it is that makes it try to unregister that hook that isn't
on the list in the first place

but on the whole I consider this issue explained and solved. I'll
continue to run with my patch on my machine (just not committed).

              Linus

^ permalink raw reply

* [PATCH net] qmi_wwan: add support for Quectel EC21 and EC25
From: Bjørn Mork @ 2016-10-10 19:12 UTC (permalink / raw)
  To: netdev; +Cc: linux-usb, Bjørn Mork

The Quectel EC21 and EC25 need the same "set DTR" request as devices
based on the MDM9230 chipset, but has no USB3 support. Our best guess
is that the "set DTR" functionality depends on chipset and/or
baseband firmware generation. But USB3 is still an optional feature.

Since we cannot enable this unconditionally for all older devices, and
there doesn't appear to be anything we can use in the USB descriptors
to identify these chips, we are forced to use a device specific quirk
flag.

Reported-and-tested-by: Sebastian Sjoholm <sebastian.sjoholm@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 9d1fce8a6e84..3ff76c6db4f6 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -59,6 +59,10 @@ enum qmi_wwan_flags {
 	QMI_WWAN_FLAG_RAWIP = 1 << 0,
 };
 
+enum qmi_wwan_quirks {
+	QMI_WWAN_QUIRK_DTR = 1 << 0,	/* needs "set DTR" request */
+};
+
 static void qmi_wwan_netdev_setup(struct net_device *net)
 {
 	struct usbnet *dev = netdev_priv(net);
@@ -411,9 +415,14 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 	 * clearing out state the clients might need.
 	 *
 	 * MDM9x30 is the first QMI chipset with USB3 support. Abuse
-	 * this fact to enable the quirk.
+	 * this fact to enable the quirk for all USB3 devices.
+	 *
+	 * There are also chipsets with the same "set DTR" requirement
+	 * but without USB3 support.  Devices based on these chips
+	 * need a quirk flag in the device ID table.
 	 */
-	if (le16_to_cpu(dev->udev->descriptor.bcdUSB) >= 0x0201) {
+	if (dev->driver_info->data & QMI_WWAN_QUIRK_DTR ||
+	    le16_to_cpu(dev->udev->descriptor.bcdUSB) >= 0x0201) {
 		qmi_wwan_manage_power(dev, 1);
 		qmi_wwan_change_dtr(dev, true);
 	}
@@ -526,6 +535,16 @@ static const struct driver_info	qmi_wwan_info = {
 	.rx_fixup       = qmi_wwan_rx_fixup,
 };
 
+static const struct driver_info	qmi_wwan_info_quirk_dtr = {
+	.description	= "WWAN/QMI device",
+	.flags		= FLAG_WWAN,
+	.bind		= qmi_wwan_bind,
+	.unbind		= qmi_wwan_unbind,
+	.manage_power	= qmi_wwan_manage_power,
+	.rx_fixup       = qmi_wwan_rx_fixup,
+	.data           = QMI_WWAN_QUIRK_DTR,
+};
+
 #define HUAWEI_VENDOR_ID	0x12D1
 
 /* map QMI/wwan function by a fixed interface number */
@@ -533,6 +552,11 @@ static const struct driver_info	qmi_wwan_info = {
 	USB_DEVICE_INTERFACE_NUMBER(vend, prod, num), \
 	.driver_info = (unsigned long)&qmi_wwan_info
 
+/* devices requiring "set DTR" quirk */
+#define QMI_QUIRK_SET_DTR(vend, prod, num) \
+	USB_DEVICE_INTERFACE_NUMBER(vend, prod, num), \
+	.driver_info = (unsigned long)&qmi_wwan_info_quirk_dtr
+
 /* Gobi 1000 QMI/wwan interface number is 3 according to qcserial */
 #define QMI_GOBI1K_DEVICE(vend, prod) \
 	QMI_FIXED_INTF(vend, prod, 3)
@@ -895,6 +919,8 @@ static const struct usb_device_id products[] = {
 	{QMI_FIXED_INTF(0x03f0, 0x4e1d, 8)},	/* HP lt4111 LTE/EV-DO/HSPA+ Gobi 4G Module */
 	{QMI_FIXED_INTF(0x22de, 0x9061, 3)},	/* WeTelecom WPD-600N */
 	{QMI_FIXED_INTF(0x1e0e, 0x9001, 5)},	/* SIMCom 7230E */
+	{QMI_QUIRK_SET_DTR(0x2c7c, 0x0125, 4)},	/* Quectel EC25, EC20 R2.0  Mini PCIe */
+	{QMI_QUIRK_SET_DTR(0x2c7c, 0x0121, 4)},	/* Quectel EC21 Mini PCIe */
 
 	/* 4. Gobi 1000 devices */
 	{QMI_GOBI1K_DEVICE(0x05c6, 0x9212)},	/* Acer Gobi Modem Device */
-- 
2.9.3

^ permalink raw reply related

* Re: slab corruption with current -git (was Re: [git pull] vfs pile 1 (splice))
From: Aaron Conole @ 2016-10-10 19:18 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Florian Westphal, Al Viro, Andrew Morton, Jens Axboe,
	Ted Ts'o, Christoph Lameter, David Miller, Pablo Neira Ayuso,
	Linux Kernel Mailing List, linux-fsdevel, Network Development,
	NetFilter
In-Reply-To: <CA+55aFy0szySf+SnysjXTyfiU=RMBo9U1sHAVaTKG=tUTF+XGw@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Mon, Oct 10, 2016 at 9:28 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So as I already answered to Dave, I'm not actually sure that this was
>> the buggy code, or that my patch would make any difference at all.
>
> My patch does seem to fix things, and in fact the warning about "hook
> not found" now triggers.
>
> So I think the bug really was that the singly-linked list handling
> code did not correctly handle the case of not finding the entry, and
> then freed (incorrectly) the last one that wasn't actually unlinked.
>
> In fact, I get quite a few warnings (56 total) about 30 seconds after
> logging in:
>
> [   54.213170] WARNING: CPU: 1 PID: 111 at net/netfilter/core.c:151
> nf_unregister_net_hook+0x8e/0x170
> ... repeat 54 times ...
> [   54.445520] WARNING: CPU: 7 PID: 111 at net/netfilter/core.c:151
> nf_unregister_net_hook+0x8e/0x170
>
> and looking in the journal, the first one is (again) immediately
> preceded by that systemd-hostnamed service stopping:
>
>   Oct 10 11:45:47 i7 audit[1546]: USER_LOGIN
>   ...
>   Oct 10 11:46:11 i7 audit[1]: SERVICE_STOP pid=1 uid=0
> auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
> msg='unit=fprintd comm="systemd" exe="/usr/lib/systemd/systemd"
> hostname=? addr=? terminal=? res=success'
>   Oct 10 11:46:13 i7 pulseaudio[1697]: [pulseaudio] bluez5-util.c:
> GetManagedObjects() failed: org.freedesktop.DBus.Error.NoReply: Did
> not receive a reply. Possible causes include: the remote application
> did not send a reply, the message bus security policy blocked the
> reply, the reply timeout expir
>   Oct 10 11:46:13 i7 dbus-daemon[1003]: [system] Failed to activate
> service 'org.bluez': timed out
>   Oct 10 11:46:20 i7 audit[1]: SERVICE_STOP pid=1 uid=0
> auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
> msg='unit=systemd-hostnamed comm="systemd"
> exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
> res=success'
>   Oct 10 11:46:20 i7 kernel: ------------[ cut here ]------------
>   Oct 10 11:46:20 i7 kernel: WARNING: CPU: 1 PID: 111 at
> net/netfilter/core.c:151 nf_unregister_net_hook+0x8e/0x170
>
> so I do think it's something to do with some network startup service
> thing (perhaps dhcp, perhaps chrome, who knows) as I do my initial
> login.
>
> David - I think that also explains what was wrong with the old code.
> In the old code, this loop:
>
>         while (hooks_entry && nf_entry_dereference(hooks_entry->next)) {
>
> would exit with "hooks_entry" pointing to the last list entry (because
> ->next was NULL). Nothing was ever unlinked in the loop itself,
> because it never actually found a matching entry, but then after the
> loop it would free that last entry because it *thought* that was the
> match.
>
> My list rewrite fixes that.
>
> Anyway, I'm assuming it will come to me from the networking tree after
> more testing by the maintainers. You can add my
>
>   Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>
> to the patch, though.
>
> David, if you want me to just commit that thing directly, I can
> obviously do so, but I do think somebody should look at
>
>  (a) that I actually got the priority list ordering right on the
>  insertion side

It looks correct.

Reviewed-by: Aaron Conole <aconole@bytheb.org>

>  (b) what it is that makes it try to unregister that hook that isn't
> on the list in the first place

This is a still problem, I think.  I wasn't able to reproduce the issue
on a fedora-23 VM.  My fedora 24 bare-metal system does trigger this,
though.  Not sure what changed in userspace/kernel interaction side (not
an excuse, but just an observation).

> but on the whole I consider this issue explained and solved. I'll
> continue to run with my patch on my machine (just not committed).

Okay.  Very sorry for this, again.

>               Linus

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox