* Re: [PATCH] ps3_gelic: Use [] to denote a flexible array member
From: David Miller @ 2019-06-18 17:43 UTC (permalink / raw)
To: geert+renesas
Cc: geoff, benh, paulus, mpe, netdev, linuxppc-dev, linux-kernel
In-Reply-To: <20190617115044.4406-1-geert+renesas@glider.be>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Mon, 17 Jun 2019 13:50:44 +0200
> Flexible array members should be denoted using [] instead of [0], else
> gcc will not warn when they are no longer at the end of a struct.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] qed: Fix -Wmaybe-uninitialized false positive
From: David Miller @ 2019-06-18 17:44 UTC (permalink / raw)
To: arnd
Cc: aelior, GR-everest-linux-l2, ariel.elior, michal.kalderon,
dan.carpenter, dbolotin, tomer.tayar, skalluru, netdev,
linux-kernel
In-Reply-To: <20190617130504.1906523-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 17 Jun 2019 15:04:49 +0200
> A previous attempt to shut up the uninitialized variable use
> warning was apparently insufficient. When CONFIG_PROFILE_ANNOTATED_BRANCHES
> is set, gcc-8 still warns, because the unlikely() check in DP_NOTICE()
> causes it to no longer track the state of all variables correctly:
>
> drivers/net/ethernet/qlogic/qed/qed_dev.c: In function 'qed_llh_set_ppfid_affinity':
> drivers/net/ethernet/qlogic/qed/qed_dev.c:798:47: error: 'abs_ppfid' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> addr = NIG_REG_PPF_TO_ENGINE_SEL + abs_ppfid * 0x4;
> ~~~~~~~~~~^~~~~
>
> This is not a nice workaround, but always initializing the output from
> qed_llh_abs_ppfid() at least shuts up the false positive reliably.
>
> Fixes: 79284adeb99e ("qed: Add llh ppfid interface and 100g support for offload protocols")
> Fixes: 8e2ea3ea9625 ("qed: Fix static checker warning")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net-next v1 08/11] xdp: tracking page_pool resources and safe removal
From: Ivan Khoronzhuk @ 2019-06-18 17:54 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Tariq Toukan, netdev@vger.kernel.org, Ilias Apalodimas,
Toke Høiland-Jørgensen, toshiaki.makita1@gmail.com,
grygorii.strashko@ti.com, mcroce@redhat.com
In-Reply-To: <20190618171951.17128ed8@carbon>
On Tue, Jun 18, 2019 at 05:19:51PM +0200, Jesper Dangaard Brouer wrote:
Hi, Jesper
>
>On Tue, 18 Jun 2019 15:54:33 +0300 Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> On Sun, Jun 16, 2019 at 10:56:25AM +0000, Tariq Toukan wrote:
>> >
>> >On 6/15/2019 12:33 PM, Ivan Khoronzhuk wrote:
>> >> On Thu, Jun 13, 2019 at 08:28:42PM +0200, Jesper Dangaard Brouer wrote:
>[...]
>> >>
>> >> What would you recommend to do for the following situation:
>> >>
>> >> Same receive queue is shared between 2 network devices. The receive ring is
>> >> filled by pages from page_pool, but you don't know the actual port (ndev)
>> >> filling this ring, because a device is recognized only after packet is
>> >> received.
>> >>
>> >> The API is so that xdp rxq is bind to network device, each frame has
>> >> reference
>> >> on it, so rxq ndev must be static. That means each netdev has it's own rxq
>> >> instance even no need in it. Thus, after your changes, page must be
>> >> returned to
>> >> the pool it was taken from, or released from old pool and recycled in
>> >> new one
>> >> somehow.
>> >>
>> >> And that is inconvenience at least. It's hard to move pages between
>> >> pools w/o performance penalty. No way to use common pool either,
>> >> as unreg_rxq now drops the pool and 2 rxqa can't reference same
>> >> pool.
>> >
>> >Within the single netdev, separate page_pool instances are anyway
>> >created for different RX rings, working under different NAPI's.
>>
>> The circumstances are so that same RX ring is shared between 2
>> netdevs... and netdev can be known only after descriptor/packet is
>> received. Thus, while filling RX ring, there is no actual device,
>> but when packet is received it has to be recycled to appropriate
>> net device pool. Before this change there were no difference from
>> which pool the page was allocated to fill RX ring, as there were no
>> owner. After this change there is owner - netdev page pool.
>
>It not really a dependency added in this patchset. A page_pool is
>strictly bound to a single RX-queue, for performance, as this allow us
>a NAPI fast-path return used for early drop (XDP_DROP).
>
>I can see that the API xdp_rxq_info_reg_mem_model() make it possible to
>call it on different xdp_rxq_info structs with the same page_pool
>pointer. But it was never intended to be used like that, and I
>consider it an API usage violation. I originally wanted to add the
>allocator pointer to xdp_rxq_info_reg() call, but the API was extended
>in different versions, so I didn't want to break users. I've actually
>tried hard to catch when drivers use the API wrong, via WARN(), but I
>guess you found a loop hole.
>
>Besides, we already have a dependency from the RX-queue to the netdev
>in the xdp_rxq_info structure.
>E.g. the xdp_rxq_info->dev is sort of
>central, and dereferenced by BPF-code to read xdp_md->ingress_ifindex,
>and also used by cpumap when creating SKBs.
Yes, That's I'm talking about.
>
>
>> For cpsw the dma unmap is common for both netdevs and no difference
>> who is freeing the page, but there is difference which pool it's
>> freed to.
>>
>> So that, while filling RX ring the page is taken from page pool of
>> ndev1, but packet is received for ndev2, it has to be later
>> returned/recycled to page pool of ndev1, but when xdp buffer is
>> handed over to xdp prog the xdp_rxq_info has reference on ndev2 ...
>>
>> And no way to predict the final ndev before packet is received, so no
>> way to choose appropriate page pool as now it becomes page owner.
>>
>> So, while RX ring filling, the page/dma recycling is needed but should
>> be some way to identify page owner only after receiving packet.
>>
>> Roughly speaking, something like:
>>
>> pool->pages_state_hold_cnt++;
>>
>> outside of page allocation API, after packet is received.
>
>Don't EVER manipulate the internal state outside of page allocation
>API. That kills the purpose of defining any API.
And I don't, that's rough propose that can be covered by newer API and here kind
of material for explanation.
This RX ring is filled by internal descriptors for 2 netdevs, it's common only
internally. But to bind descriptor with a page, the page must be allocated from
pool bind to ndev, In case of cpsw, ndev is identified only after the
descriptor/packet is received. Then, it can be related to some ndev and thus
pool. The example in question only shows what API needs to allow underneath.
For this, the following is needed:
1) Allocate page from page pool, but w/o counter inc, so driver can became the
owner and bind it with a descriptor and put it to RX ring for receiving a
packet by h/w.
2) after a packet is received the counter needs to be inc for a pool the packet
is destined for and let xdp infrastructure do the rest.
That's it.
I don't propose to change the API for those who doesn't require it,
just let to use more detailed variant for those who can't use it in regular way.
And it's not final decision would be nice to hear another constructive
propositions.
>
>> and free of the counter while allocation (w/o owing the page).
>
>You use-case of two netdev's sharing the same RX-queue sounds dubious,
>and very hardware specific. I'm not sure why we want to bend the APIs
>to support this?
The question is rather why not? The allocator is supposed to be used as one of
the main generic solutions for XDP, if not say more, it should be able to cover
most of the hardware, not only for hi speed solutions, but for other usecases.
I don't propose to change main API, left it as is, just extend it that not only
single ndev hardware can use it... I can add it myself if you don't want to
bother.
> If we had to allow page_pool to be registered twice, via
>xdp_rxq_info_reg_mem_model() then I guess we could extend page_pool
>with a usage/users reference count, and then only really free the
>page_pool when refcnt reach zero. But it just seems and looks wrong
>(in the code) as the hole trick to get performance is to only have one
>user.
I don't propose this.
>
>--
>Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net v2] tun: wake up waitqueues after IFF_UP is set
From: David Miller @ 2019-06-18 17:55 UTC (permalink / raw)
To: lifei.shirley; +Cc: jasowang, netdev, linux-kernel, zhengfeiran, duanxiongchun
In-Reply-To: <20190617132636.72496-1-lifei.shirley@bytedance.com>
From: Fei Li <lifei.shirley@bytedance.com>
Date: Mon, 17 Jun 2019 21:26:36 +0800
> Currently after setting tap0 link up, the tun code wakes tx/rx waited
> queues up in tun_net_open() when .ndo_open() is called, however the
> IFF_UP flag has not been set yet. If there's already a wait queue, it
> would fail to transmit when checking the IFF_UP flag in tun_sendmsg().
> Then the saving vhost_poll_start() will add the wq into wqh until it
> is waken up again. Although this works when IFF_UP flag has been set
> when tun_chr_poll detects; this is not true if IFF_UP flag has not
> been set at that time. Sadly the latter case is a fatal error, as
> the wq will never be waken up in future unless later manually
> setting link up on purpose.
>
> Fix this by moving the wakeup process into the NETDEV_UP event
> notifying process, this makes sure IFF_UP has been set before all
> waited queues been waken up.
>
> Signed-off-by: Fei Li <lifei.shirley@bytedance.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
From: Benedikt Spranger @ 2019-06-18 17:57 UTC (permalink / raw)
To: netdev; +Cc: Benedikt Spranger
Hi,
while puting a Banana Pi R1 board into operation I faced network hickups
and get into serious trouble from my coworkers:
Banana Pi network setup:
PC (eth1) <--> BPi R1 (wan) / BPi R1 (lan4) <--> DUT (eth0)
10.0.32.1 10.0.32.2 172.16.0.1 172.16.0.2
---8<---
#! /bin/bash
# create VLANs
ip link add link eth0 name eth0.1 type vlan id 1
ip link add link eth0 name eth0.100 type vlan id 100
# create bridges
ip link add br0 type bridge
ip link set dev br0 type bridge vlan_filtering 1
ip link add br1 type bridge
ip link set dev br1 type bridge vlan_filtering 1
# add interfaces to bridges
ip link set lan1 master br0
ip link set lan2 master br0
ip link set lan3 master br0
ip link set lan4 master br0
ip link set eth0.100 master br0
ip link set wan master br1
# adjust tagging
bridge vlan add dev lan1 vid 100 pvid untagged
bridge vlan add dev lan2 vid 100 pvid untagged
bridge vlan add dev lan3 vid 100 pvid untagged
bridge vlan add dev lan4 vid 100 pvid untagged
bridge vlan del dev lan1 vid 1
bridge vlan del dev lan2 vid 1
bridge vlan del dev lan3 vid 1
bridge vlan del dev lan4 vid 1
# set IPs
ip addr add 10.0.32.2/24 dev eth0.1
ip addr add 172.16.0.1/24 dev br0
# up and run
ip link set eth0 up
ip link set eth0.1 up
ip link set eth0.100 up
ip link set br0 up
ip link set br1 up
ip link set wan up
ip link set lan1 up
ip link set lan2 up
ip link set lan3 up
ip link set lan4 up
---8<---
While trying to ping a non existing host 10.0.32.111 result in
doublication of ARP broadcasts:
On the BPi R1:
# tshark -i br0
1 8.157471671 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
Tell 10.0.32.1
2 9.167563213 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
Tell 10.0.32.1
3 10.191703047 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
4 11.215905797 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
5 12.243932964 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
6 13.264033298 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
# tshark -i wan0
1 8.157421295 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
Tell 10.0.32.1
2 9.167510087 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
Tell 10.0.32.1
3 10.191649213 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
4 11.215856838 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
5 12.243881922 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
6 13.263981506 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
On the PC:
# tshark -i eth1
116 272.660717059 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
117 272.661062292 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
118 273.670690338 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
119 273.671058605 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
120 274.694720511 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
121 274.695250723 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
122 275.718813538 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
123 275.719285852 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
124 276.746729795 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
125 276.747236341 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
126 277.766722429 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
127 277.767233098 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
Choosing between network disturbance by mirrored broadcast packages (and angry
coworkers) or lack of multicast, I choose the later.
Regards
Bene Spranger
Florian Fainelli (1):
net: dsa: b53: Turn on managed mode and set IMP port
Sebastian Andrzej Siewior (1):
net: dsa: b53: enbale broadcom tags on bcm531x5
drivers/net/dsa/b53/b53_common.c | 21 ++++++++++++++++-----
drivers/net/dsa/bcm_sf2.c | 3 ---
2 files changed, 16 insertions(+), 8 deletions(-)
--
2.19.0
^ permalink raw reply
* [RFC PATCH 2/2] net: dsa: b53: enbale broadcom tags on bcm531x5
From: Benedikt Spranger @ 2019-06-18 17:57 UTC (permalink / raw)
To: netdev; +Cc: Sebastian Andrzej Siewior, Benedikt Spranger
In-Reply-To: <20190618175712.71148-1-b.spranger@linutronix.de>
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
The broadcom tags are required to distinguish the traffic from external
ports and not use a bridge like setup (which is currently broken because
managed mode is now enabled). The tagged mode works for broadcast and
unicast traffic. The multicast missis are not handled.
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
---
drivers/net/dsa/b53/b53_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 06b9a7a81ae0..10f1ece64104 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1793,7 +1793,7 @@ enum dsa_tag_protocol b53_get_tag_protocol(struct dsa_switch *ds, int port)
* mode to be turned on which means we need to specifically manage ARL
* misses on multicast addresses (TBD).
*/
- if (is5325(dev) || is5365(dev) || is539x(dev) || is531x5(dev) ||
+ if (is5325(dev) || is5365(dev) || is539x(dev) ||
!b53_can_enable_brcm_tags(ds, port))
return DSA_TAG_PROTO_NONE;
--
2.19.0
^ permalink raw reply related
* [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port
From: Benedikt Spranger @ 2019-06-18 17:57 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, Benedikt Spranger
In-Reply-To: <20190618175712.71148-1-b.spranger@linutronix.de>
From: Florian Fainelli <f.fainelli@gmail.com>
When enabling Broadcom tags on earlier devices such as BCM53125, we need
to also enable Managed mode, as well as configure which port is going to
be the IMP port. If we did not do that, the switch would just pass
through the Broadcom tags on the wire and not act on them. We need to
have bcm_sf2 stop overwriting the SWMODE register and let b53 deal with
that now.
Fixes: 7edc58d614d4 ("net: dsa: b53: Turn on Broadcom tags")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
---
drivers/net/dsa/b53/b53_common.c | 19 +++++++++++++++----
drivers/net/dsa/bcm_sf2.c | 3 ---
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index c8040ecf4425..06b9a7a81ae0 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -331,9 +331,9 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
b53_read8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, &mgmt);
if (enable)
- mgmt |= SM_SW_FWD_EN;
+ mgmt |= SM_SW_FWD_EN | SM_SW_FWD_MODE;
else
- mgmt &= ~SM_SW_FWD_EN;
+ mgmt &= ~(SM_SW_FWD_EN | SM_SW_FWD_MODE);
b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, mgmt);
@@ -364,8 +364,6 @@ static void b53_enable_vlan(struct b53_device *dev, bool enable,
b53_read8(dev, B53_VLAN_PAGE, B53_VLAN_CTRL5, &vc5);
}
- mgmt &= ~SM_SW_FWD_MODE;
-
if (enable) {
vc0 |= VC0_VLAN_EN | VC0_VID_CHK_EN | VC0_VID_HASH_VID;
vc1 |= VC1_RX_MCST_UNTAG_EN | VC1_RX_MCST_FWD_EN;
@@ -589,6 +587,19 @@ void b53_brcm_hdr_setup(struct dsa_switch *ds, int port)
hdr_ctl &= ~val;
b53_write8(dev, B53_MGMT_PAGE, B53_BRCM_HDR, hdr_ctl);
+ /* Set IMP port number, necessary for Broadcom tags to be used
+ * while in managed mode
+ */
+ if (tag_en) {
+ b53_read8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, &hdr_ctl);
+ hdr_ctl &= ~GC_FRM_MGMT_PORT_M;
+ val = 0;
+ if (port == 8)
+ val = GC_FRM_MGMT_PORT_MII;
+ hdr_ctl |= val;
+ b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, hdr_ctl);
+ }
+
/* Registers below are only accessible on newer devices */
if (!is58xx(dev))
return;
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 3811fdbda13e..47f55dab14f6 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -53,9 +53,6 @@ static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
reg &= ~(RX_DIS | TX_DIS);
core_writel(priv, reg, CORE_IMP_CTL);
- /* Enable forwarding */
- core_writel(priv, SW_FWDG_EN, CORE_SWMODE);
-
/* Enable IMP port in dumb mode */
reg = core_readl(priv, CORE_SWITCH_CTRL);
reg |= MII_DUMB_FWDG_EN;
--
2.19.0
^ permalink raw reply related
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Dan Williams @ 2019-06-18 18:06 UTC (permalink / raw)
To: Alex Elder, Johannes Berg, Arnd Bergmann
Cc: Subash Abhinov Kasiviswanathan, abhishek.esse, Ben Chan,
Bjorn Andersson, cpratapa, David Miller, DTML, Eric Caruso,
evgreen, Ilias Apalodimas, Linux ARM, linux-arm-msm,
Linux Kernel Mailing List, linux-soc, Networking, syadagir
In-Reply-To: <850eed1d-0fec-c396-6e91-b5f1f8440ded@linaro.org>
On Tue, 2019-06-18 at 10:20 -0500, Alex Elder wrote:
> On 6/17/19 7:25 AM, Johannes Berg wrote:
> > On Mon, 2019-06-17 at 13:42 +0200, Johannes Berg wrote:
> >
> > > But anyway, as I alluded to above, I had something like this in
> > > mind:
> >
> > I forgot to state this here, but this was *heavily* influenced by
> > discussions with Dan - many thanks to him.
>
> Thanks for getting even more concrete with this. Code is the
> most concise way of describing things, once the general ideas
> seem to be coming together.
>
> I'm not going to comment on the specific code bits, but I have
> some more general questions and comments on the design. Some
> of these are simply due to my lack of knowledge of how WWAN/modem
> interactions normally work.
>
> First, a few terms (correct or improve as you like):
> - WWAN device is a hardware device (like IPA) that presents a
> connection between AP and modem, and presents an interface
> that allows the use of that connection to be managed.
> - WWAN netdevice represents a Linux network interface, with its
> operations and queues, etc., but implements a standardized
> set of WWAN-specific operations. It represents a logical
> ' channel whose data is multiplexed over the WWAN device.
> - WWAN channel is a user space abstraction that corresponds
> with a WWAN netdevice (but I'm not clear on all the ways
> they differ or interact).
When Johannes and I have talked about "WWAN channel" we mean a control
or data or other channel. That could be QMI, AT, MBIM control, GPS,
PCSC, QMAP, MBIM data, PPP TTY, DM/DIAG, CDC-ETHER, CDC-NCM, Sierra
HIP, etc. Or even voice-call audio :)
A netdev is a Linux abstraction of a WWAN *data* channel, be that QMI
or CDC-ETHER or whatever.
> - The WWAN core is kernel code that presents abstractions
> for WWAN devices and netdevices, so they can be managed
> in a generic way. It is for configuration and communication
> and is not at all involved in the data path.
>
> You're saying that the WWAN driver space calls wwan_add()
> to register itself as a new WWAN device.
>
> You're also saying that a WWAN device "attaches" a WWAN
> netdevice, which is basically notifying the WWAN core
> that the new netdev/channel is available for use.
> - I trust that a "tentative" attachement is necessary. But
> I'm not sure what makes it transition into becoming a
> "real" one, or how that event gets communicated.
Linux usually tries to keep drivers generic and focused; each driver is
written for a specific function. For example, a USB device usually
provides multiple USB interfaces which will be bound to different Linux
drivers like a TTY, cdc-ether, QMI (via qmi_wwan), cdc-acm, etc.
These drivers are often generic and we may not have enough information
in one driver to know that the parent of this interface is a WWAN
device. But another driver might. Since probing is asynchronous we may
have cdc-acm bind to a device and provide a TTY before cdc-ether (which
does know it's a WWAN) binds and provides the netdevice.
> Some questions:
> - What causes a new channel to be created? Is it initiated
> by the WWAN device driver? Does the modem request that
> it get created? User space? Both?
Either created at driver bind time in the kernel (usually control
channels) or initiated by userspace when the WWAN management process
has coordinated with the firmware for another channel. Honestly
userspace should probably always create the netdevices (since they are
always useless until userspace coordinates with the firmware about
them) but that's not how things are yet.
[ A concrete example...
Assume a QMI device has an existing packet data connection which is
abstracted by a netdevice on the Linux side. Now the WWAN management
daemon wants to create a second packet data connection with a different
APN (maybe an MMS connection, maybe a VOIP one, maybe an IPv6). It
sends a WDS Start Network request to the modem firmware and receives a
new QMI Packet Data Handle.
The management daemon must somehow get a netdevice associated with this
new Packet Data Handle. It would ask the WWAN kernel device to create a
new data channel with the PDH, and would get back the ifindex of that
netdevice which it would configure with the IP that it gets from the
firmware via the WDS Get Current Settings QMI request.
The WWAN device would forward the request down to IPA (or rmnet) which
would then create the netdevice using the PDH as the QMAP MUX ID for
that netdevice's traffic.]
> - What causes a created channel to be removed?
Driver removal, userspace WWAN daemon terminating the packet data
connection which the channel represents, the modem terminating the
packet data connection (eg network initiated disconnect), etc.
> - You distinguish between attaching a netdevice and (what
> I'll call) activating it. What causes activation?
Can you describe what you mean by "activating"? Do you mean
successfully TX/RX packets via the netdev and the outside world?
I read "attach" here as simply associating an existing netdev with the
"parent" WWAN device. A purely Linux operation that is only book-
keeping and may not have any interaction with the modem.
> - How are the attributes of a WWAN device or channel set,
> or communicated?
Via netlink attributes when userspace asks the WWAN device to create a
new channel. In the control methods I've seen, only userspace really
knows the channel identifier that it and the modem have agreed on (eg
what the MUX ID in the QMAP header would be, or the MBIM Session ID).
> - Are there any attributes that are only optionally supported,
> and if so, how are the supported ones communicated?
Yeah, capabilities would be important here and I don't think Johannes
accounted for that yet.
> - Which WWAN channel attributes must be set *before* the
> channel is activated, and can't be changed? Are there any
> that can be changed dynamically?
I would assume userspace must pass the agreed identifier (QMUX ID, MBIM
session ID, etc) when creating the channel and that wouldn't change. I
think a world where you can dynamically change the MUX ID/SessionID/etc
is a more complicated one.
Things like QoS could change but I don't recall if modems allow that;
eg does a +CGEQOS (or equivalent QMI WDS LTE QoS Parameters request)
take effect while the bearer is active, or is it only respected on
bearer creation?
> And while the whole point of this is to make things generic,
> it might be nice to have a way to implement a new feature
> before it can be "standardized".
That would be nice, but I'd rather have the conversation about if/how
to standardize things before they make it into the kernel and have
their API set in stone... which is how we ended up with 5 ways of doing
the same thing already.
Dan
> Thanks.
>
> -Alex
>
> PS I don't want to exclude anybody but we could probably start
> a different mail chain on this topic...
>
> > > driver_dev
> > > struct device *dev (USB, PCI, ...)
> > > net_device NA
> > > net_device NB
> > > tty TA
> > > ...
> > >
>
> . . .
^ permalink raw reply
* [PATCH net-next 0/2] inet: fix defrag units dismantle races
From: Eric Dumazet @ 2019-06-18 18:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
This series add a new pre_exit() method to struct pernet_operations
to solve a race in defrag units dismantle, without adding extra
delays to netns dismantles.
Eric Dumazet (2):
netns: add pre_exit method to struct pernet_operations
inet: fix various use-after-free in defrags units
include/net/inet_frag.h | 8 ++++++-
include/net/ipv6_frag.h | 2 ++
include/net/net_namespace.h | 5 +++++
net/core/net_namespace.c | 28 +++++++++++++++++++++++++
net/ieee802154/6lowpan/reassembly.c | 13 ++++++++++--
net/ipv4/inet_fragment.c | 19 ++++-------------
net/ipv4/ip_fragment.c | 14 +++++++++++--
net/ipv6/netfilter/nf_conntrack_reasm.c | 10 +++++++--
net/ipv6/reassembly.c | 10 +++++++--
9 files changed, 85 insertions(+), 24 deletions(-)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* [PATCH net-next 1/2] netns: add pre_exit method to struct pernet_operations
From: Eric Dumazet @ 2019-06-18 18:08 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20190618180900.88939-1-edumazet@google.com>
Current struct pernet_operations exit() handlers are highly
discouraged to call synchronize_rcu().
There are cases where we need them, and exit_batch() does
not help the common case where a single netns is dismantled.
This patch leverages the existing synchronize_rcu() call
in cleanup_net()
Calling optional ->pre_exit() method before ->exit() or
->exit_batch() allows to benefit from a single synchronize_rcu()
call.
Note that the synchronize_rcu() calls added in this patch
are only in error paths or slow paths.
Tested:
$ time for i in {1..1000}; do unshare -n /bin/false;done
real 0m2.612s
user 0m0.171s
sys 0m2.216s
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/net_namespace.h | 5 +++++
net/core/net_namespace.c | 28 ++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index abb4f92456e1b0a0eb59bba676295d202ac008e0..ad9243afac672d054b27b933a23a696232987b61 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -355,8 +355,13 @@ struct pernet_operations {
* synchronize_rcu() related to these pernet_operations,
* instead of separate synchronize_rcu() for every net.
* Please, avoid synchronize_rcu() at all, where it's possible.
+ *
+ * Note that a combination of pre_exit() and exit() can
+ * be used, since a synchronize_rcu() is guaranteed between
+ * the calls.
*/
int (*init)(struct net *net);
+ void (*pre_exit)(struct net *net);
void (*exit)(struct net *net);
void (*exit_batch)(struct list_head *net_exit_list);
unsigned int *id;
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 15f68842ac6b4c32845d9550f2b2e89ca4406999..89dc99a28978f14f8227904013282212b15d513b 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -145,6 +145,17 @@ static void ops_free(const struct pernet_operations *ops, struct net *net)
}
}
+static void ops_pre_exit_list(const struct pernet_operations *ops,
+ struct list_head *net_exit_list)
+{
+ struct net *net;
+
+ if (ops->pre_exit) {
+ list_for_each_entry(net, net_exit_list, exit_list)
+ ops->pre_exit(net);
+ }
+}
+
static void ops_exit_list(const struct pernet_operations *ops,
struct list_head *net_exit_list)
{
@@ -328,6 +339,12 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
* for the pernet modules whose init functions did not fail.
*/
list_add(&net->exit_list, &net_exit_list);
+ saved_ops = ops;
+ list_for_each_entry_continue_reverse(ops, &pernet_list, list)
+ ops_pre_exit_list(ops, &net_exit_list);
+
+ synchronize_rcu();
+
saved_ops = ops;
list_for_each_entry_continue_reverse(ops, &pernet_list, list)
ops_exit_list(ops, &net_exit_list);
@@ -541,10 +558,15 @@ static void cleanup_net(struct work_struct *work)
list_add_tail(&net->exit_list, &net_exit_list);
}
+ /* Run all of the network namespace pre_exit methods */
+ list_for_each_entry_reverse(ops, &pernet_list, list)
+ ops_pre_exit_list(ops, &net_exit_list);
+
/*
* Another CPU might be rcu-iterating the list, wait for it.
* This needs to be before calling the exit() notifiers, so
* the rcu_barrier() below isn't sufficient alone.
+ * Also the pre_exit() and exit() methods need this barrier.
*/
synchronize_rcu();
@@ -1101,6 +1123,8 @@ static int __register_pernet_operations(struct list_head *list,
out_undo:
/* If I have an error cleanup all namespaces I initialized */
list_del(&ops->list);
+ ops_pre_exit_list(ops, &net_exit_list);
+ synchronize_rcu();
ops_exit_list(ops, &net_exit_list);
ops_free_list(ops, &net_exit_list);
return error;
@@ -1115,6 +1139,8 @@ static void __unregister_pernet_operations(struct pernet_operations *ops)
/* See comment in __register_pernet_operations() */
for_each_net(net)
list_add_tail(&net->exit_list, &net_exit_list);
+ ops_pre_exit_list(ops, &net_exit_list);
+ synchronize_rcu();
ops_exit_list(ops, &net_exit_list);
ops_free_list(ops, &net_exit_list);
}
@@ -1139,6 +1165,8 @@ static void __unregister_pernet_operations(struct pernet_operations *ops)
} else {
LIST_HEAD(net_exit_list);
list_add(&init_net.exit_list, &net_exit_list);
+ ops_pre_exit_list(ops, &net_exit_list);
+ synchronize_rcu();
ops_exit_list(ops, &net_exit_list);
ops_free_list(ops, &net_exit_list);
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net-next 2/2] inet: fix various use-after-free in defrags units
From: Eric Dumazet @ 2019-06-18 18:09 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, syzbot
In-Reply-To: <20190618180900.88939-1-edumazet@google.com>
syzbot reported another issue caused by my recent patches. [1]
The issue here is that fqdir_exit() is initiating a work queue
and immediately returns. A bit later cleanup_net() was able
to free the MIB (percpu data) and the whole struct net was freed,
but we had active frag timers that fired and triggered use-after-free.
We need to make sure that timers can catch fqdir->dead being set,
to bailout.
Since RCU is used for the reader side, this means
we want to respect an RCU grace period between these operations :
1) qfdir->dead = 1;
2) netns dismantle (freeing of various data structure)
This patch uses new new (struct pernet_operations)->pre_exit
infrastructure to ensures a full RCU grace period
happens between fqdir_pre_exit() and fqdir_exit()
This also means we can use a regular work queue, we no
longer need rcu_work.
Tested:
$ time for i in {1..1000}; do unshare -n /bin/false;done
real 0m2.585s
user 0m0.160s
sys 0m2.214s
[1]
BUG: KASAN: use-after-free in ip_expire+0x73e/0x800 net/ipv4/ip_fragment.c:152
Read of size 8 at addr ffff88808b9fe330 by task syz-executor.4/11860
CPU: 1 PID: 11860 Comm: syz-executor.4 Not tainted 5.2.0-rc2+ #22
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.cold+0x7c/0x20d mm/kasan/report.c:188
__kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
kasan_report+0x12/0x20 mm/kasan/common.c:614
__asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
ip_expire+0x73e/0x800 net/ipv4/ip_fragment.c:152
call_timer_fn+0x193/0x720 kernel/time/timer.c:1322
expire_timers kernel/time/timer.c:1366 [inline]
__run_timers kernel/time/timer.c:1685 [inline]
__run_timers kernel/time/timer.c:1653 [inline]
run_timer_softirq+0x66f/0x1740 kernel/time/timer.c:1698
__do_softirq+0x25c/0x94c kernel/softirq.c:293
invoke_softirq kernel/softirq.c:374 [inline]
irq_exit+0x180/0x1d0 kernel/softirq.c:414
exiting_irq arch/x86/include/asm/apic.h:536 [inline]
smp_apic_timer_interrupt+0x13b/0x550 arch/x86/kernel/apic/apic.c:1068
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:806
</IRQ>
RIP: 0010:tomoyo_domain_quota_is_ok+0x131/0x540 security/tomoyo/util.c:1035
Code: 24 4c 3b 65 d0 0f 84 9c 00 00 00 e8 19 1d 73 fe 49 8d 7c 24 18 48 ba 00 00 00 00 00 fc ff df 48 89 f8 48 c1 e8 03 0f b6 04 10 <48> 89 fa 83 e2 07 38 d0 7f 08 84 c0 0f 85 69 03 00 00 41 0f b6 5c
RSP: 0018:ffff88806ae079c0 EFLAGS: 00000a02 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000000 RBX: 0000000000000010 RCX: ffffc9000e655000
RDX: dffffc0000000000 RSI: ffffffff82fd88a7 RDI: ffff888086202398
RBP: ffff88806ae07a00 R08: ffff88808b6c8700 R09: ffffed100d5c0f4d
R10: ffffed100d5c0f4c R11: 0000000000000000 R12: ffff888086202380
R13: 0000000000000030 R14: 00000000000000d3 R15: 0000000000000000
tomoyo_supervisor+0x2e8/0xef0 security/tomoyo/common.c:2087
tomoyo_audit_path_number_log security/tomoyo/file.c:235 [inline]
tomoyo_path_number_perm+0x42f/0x520 security/tomoyo/file.c:734
tomoyo_file_ioctl+0x23/0x30 security/tomoyo/tomoyo.c:335
security_file_ioctl+0x77/0xc0 security/security.c:1370
ksys_ioctl+0x57/0xd0 fs/ioctl.c:711
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4592c9
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f8db5e44c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004592c9
RDX: 0000000020000080 RSI: 00000000000089f1 RDI: 0000000000000006
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f8db5e456d4
R13: 00000000004cc770 R14: 00000000004d5cd8 R15: 00000000ffffffff
Allocated by task 9047:
save_stack+0x23/0x90 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_kmalloc mm/kasan/common.c:489 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:462
kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:497
slab_post_alloc_hook mm/slab.h:437 [inline]
slab_alloc mm/slab.c:3326 [inline]
kmem_cache_alloc+0x11a/0x6f0 mm/slab.c:3488
kmem_cache_zalloc include/linux/slab.h:732 [inline]
net_alloc net/core/net_namespace.c:386 [inline]
copy_net_ns+0xed/0x340 net/core/net_namespace.c:426
create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:107
unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:206
ksys_unshare+0x440/0x980 kernel/fork.c:2692
__do_sys_unshare kernel/fork.c:2760 [inline]
__se_sys_unshare kernel/fork.c:2758 [inline]
__x64_sys_unshare+0x31/0x40 kernel/fork.c:2758
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 2541:
save_stack+0x23/0x90 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/common.c:451
kasan_slab_free+0xe/0x10 mm/kasan/common.c:459
__cache_free mm/slab.c:3432 [inline]
kmem_cache_free+0x86/0x260 mm/slab.c:3698
net_free net/core/net_namespace.c:402 [inline]
net_drop_ns.part.0+0x70/0x90 net/core/net_namespace.c:409
net_drop_ns net/core/net_namespace.c:408 [inline]
cleanup_net+0x538/0x960 net/core/net_namespace.c:571
process_one_work+0x989/0x1790 kernel/workqueue.c:2269
worker_thread+0x98/0xe40 kernel/workqueue.c:2415
kthread+0x354/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff88808b9fe100
which belongs to the cache net_namespace of size 6784
The buggy address is located 560 bytes inside of
6784-byte region [ffff88808b9fe100, ffff88808b9ffb80)
The buggy address belongs to the page:
page:ffffea00022e7f80 refcount:1 mapcount:0 mapping:ffff88821b6f60c0 index:0x0 compound_mapcount: 0
flags: 0x1fffc0000010200(slab|head)
raw: 01fffc0000010200 ffffea000256f288 ffffea0001bbef08 ffff88821b6f60c0
raw: 0000000000000000 ffff88808b9fe100 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88808b9fe200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88808b9fe280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88808b9fe300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88808b9fe380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88808b9fe400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
Fixes: 3c8fc8782044 ("inet: frags: rework rhashtable dismantle")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
include/net/inet_frag.h | 8 +++++++-
include/net/ipv6_frag.h | 2 ++
net/ieee802154/6lowpan/reassembly.c | 13 +++++++++++--
net/ipv4/inet_fragment.c | 19 ++++---------------
net/ipv4/ip_fragment.c | 14 ++++++++++++--
net/ipv6/netfilter/nf_conntrack_reasm.c | 10 ++++++++--
net/ipv6/reassembly.c | 10 ++++++++--
7 files changed, 52 insertions(+), 24 deletions(-)
diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index e91b79ad4e4adfc1dadb9d00a91f2f2c669d732d..46574d996f1dc658487283de2ca469f1db42be03 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -20,7 +20,7 @@ struct fqdir {
/* Keep atomic mem on separate cachelines in structs that include it */
atomic_long_t mem ____cacheline_aligned_in_smp;
- struct rcu_work destroy_rwork;
+ struct work_struct destroy_work;
};
/**
@@ -113,6 +113,12 @@ int inet_frags_init(struct inet_frags *);
void inet_frags_fini(struct inet_frags *);
int fqdir_init(struct fqdir **fqdirp, struct inet_frags *f, struct net *net);
+
+static void inline fqdir_pre_exit(struct fqdir *fqdir)
+{
+ fqdir->high_thresh = 0; /* prevent creation of new frags */
+ fqdir->dead = true;
+}
void fqdir_exit(struct fqdir *fqdir);
void inet_frag_kill(struct inet_frag_queue *q);
diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h
index 1f77fb4dc79df6bc4e41d6d2f4d49ace32082ca4..a21e8b1381a107c8afd4a832f523bc87d045b890 100644
--- a/include/net/ipv6_frag.h
+++ b/include/net/ipv6_frag.h
@@ -67,6 +67,8 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
struct sk_buff *head;
rcu_read_lock();
+ if (fq->q.fqdir->dead)
+ goto out_rcu_unlock;
spin_lock(&fq->q.lock);
if (fq->q.flags & INET_FRAG_COMPLETE)
diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
index a0ed13cd120e783114bf75309006b6d04901cd63..e4aba5d485be6a59c639a23b75172f5015c4280f 100644
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -459,6 +459,14 @@ static int __net_init lowpan_frags_init_net(struct net *net)
return res;
}
+static void __net_exit lowpan_frags_pre_exit_net(struct net *net)
+{
+ struct netns_ieee802154_lowpan *ieee802154_lowpan =
+ net_ieee802154_lowpan(net);
+
+ fqdir_pre_exit(ieee802154_lowpan->fqdir);
+}
+
static void __net_exit lowpan_frags_exit_net(struct net *net)
{
struct netns_ieee802154_lowpan *ieee802154_lowpan =
@@ -469,8 +477,9 @@ static void __net_exit lowpan_frags_exit_net(struct net *net)
}
static struct pernet_operations lowpan_frags_ops = {
- .init = lowpan_frags_init_net,
- .exit = lowpan_frags_exit_net,
+ .init = lowpan_frags_init_net,
+ .pre_exit = lowpan_frags_pre_exit_net,
+ .exit = lowpan_frags_exit_net,
};
static u32 lowpan_key_hashfn(const void *data, u32 len, u32 seed)
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index 5c25727d491e75bec4c31457784676d90e29e80a..d666756be5f18404ce8e85a95e9f09636d5f2694 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -145,10 +145,9 @@ static void inet_frags_free_cb(void *ptr, void *arg)
inet_frag_destroy(fq);
}
-static void fqdir_rwork_fn(struct work_struct *work)
+static void fqdir_work_fn(struct work_struct *work)
{
- struct fqdir *fqdir = container_of(to_rcu_work(work),
- struct fqdir, destroy_rwork);
+ struct fqdir *fqdir = container_of(work, struct fqdir, destroy_work);
struct inet_frags *f = fqdir->f;
rhashtable_free_and_destroy(&fqdir->rhashtable, inet_frags_free_cb, NULL);
@@ -187,18 +186,8 @@ EXPORT_SYMBOL(fqdir_init);
void fqdir_exit(struct fqdir *fqdir)
{
- fqdir->high_thresh = 0; /* prevent creation of new frags */
-
- fqdir->dead = true;
-
- /* call_rcu is supposed to provide memory barrier semantics,
- * separating the setting of fqdir->dead with the destruction
- * work. This implicit barrier is paired with inet_frag_kill().
- */
-
- INIT_RCU_WORK(&fqdir->destroy_rwork, fqdir_rwork_fn);
- queue_rcu_work(system_wq, &fqdir->destroy_rwork);
-
+ INIT_WORK(&fqdir->destroy_work, fqdir_work_fn);
+ queue_work(system_wq, &fqdir->destroy_work);
}
EXPORT_SYMBOL(fqdir_exit);
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 1ffaec056821b8e0bd0915403b0a3a1d449690ec..4385eb9e781ffc017a65d166ddf12c11fc901c0e 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -143,6 +143,10 @@ static void ip_expire(struct timer_list *t)
net = qp->q.fqdir->net;
rcu_read_lock();
+
+ if (qp->q.fqdir->dead)
+ goto out_rcu_unlock;
+
spin_lock(&qp->q.lock);
if (qp->q.flags & INET_FRAG_COMPLETE)
@@ -676,6 +680,11 @@ static int __net_init ipv4_frags_init_net(struct net *net)
return res;
}
+static void __net_exit ipv4_frags_pre_exit_net(struct net *net)
+{
+ fqdir_pre_exit(net->ipv4.fqdir);
+}
+
static void __net_exit ipv4_frags_exit_net(struct net *net)
{
ip4_frags_ns_ctl_unregister(net);
@@ -683,8 +692,9 @@ static void __net_exit ipv4_frags_exit_net(struct net *net)
}
static struct pernet_operations ip4_frags_ops = {
- .init = ipv4_frags_init_net,
- .exit = ipv4_frags_exit_net,
+ .init = ipv4_frags_init_net,
+ .pre_exit = ipv4_frags_pre_exit_net,
+ .exit = ipv4_frags_exit_net,
};
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index b8962d4d6ae650f890fec8de754d4ff92a050974..3299a389d166b69467e741813822f8182321ce39 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -499,6 +499,11 @@ static int nf_ct_net_init(struct net *net)
return res;
}
+static void nf_ct_net_pre_exit(struct net *net)
+{
+ fqdir_pre_exit(net->nf_frag.fqdir);
+}
+
static void nf_ct_net_exit(struct net *net)
{
nf_ct_frags6_sysctl_unregister(net);
@@ -506,8 +511,9 @@ static void nf_ct_net_exit(struct net *net)
}
static struct pernet_operations nf_ct_net_ops = {
- .init = nf_ct_net_init,
- .exit = nf_ct_net_exit,
+ .init = nf_ct_net_init,
+ .pre_exit = nf_ct_net_pre_exit,
+ .exit = nf_ct_net_exit,
};
static const struct rhashtable_params nfct_rhash_params = {
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 0c9fd8a7c4e726c2be8c7831f9b32f728c3b973c..ca05b16f1bb95d4122a79fa9759011fd204f3e6f 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -520,6 +520,11 @@ static int __net_init ipv6_frags_init_net(struct net *net)
return res;
}
+static void __net_exit ipv6_frags_pre_exit_net(struct net *net)
+{
+ fqdir_pre_exit(net->ipv6.fqdir);
+}
+
static void __net_exit ipv6_frags_exit_net(struct net *net)
{
ip6_frags_ns_sysctl_unregister(net);
@@ -527,8 +532,9 @@ static void __net_exit ipv6_frags_exit_net(struct net *net)
}
static struct pernet_operations ip6_frags_ops = {
- .init = ipv6_frags_init_net,
- .exit = ipv6_frags_exit_net,
+ .init = ipv6_frags_init_net,
+ .pre_exit = ipv6_frags_pre_exit_net,
+ .exit = ipv6_frags_exit_net,
};
static const struct rhashtable_params ip6_rhash_params = {
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: [PATCH] netfilter: nf_conntrack_sip: fix IPV6 dependency
From: Arnd Bergmann @ 2019-06-18 18:09 UTC (permalink / raw)
To: Alin Năstac
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Máté Eckl, Fernando Fernandez Mancera,
netfilter-devel, coreteam, Networking, Linux Kernel Mailing List
In-Reply-To: <CAF1oqRAuk7h2Z2iheq3Ze1vTMNWLf5HHn83fZt07hXA4nOPbAg@mail.gmail.com>
On Fri, Mar 8, 2019 at 5:23 PM Alin Năstac <alin.nastac@gmail.com> wrote:
> On Fri, Mar 8, 2019 at 5:04 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Mon, Mar 04, 2019 at 09:40:12PM +0100, Arnd Bergmann wrote:
> > > With CONFIG_IPV6=m and CONFIG_NF_CONNTRACK_SIP=y, we now get a link failure:
> > >
> > > net/netfilter/nf_conntrack_sip.o: In function `process_sdp':
> > > nf_conntrack_sip.c:(.text+0x4344): undefined reference to `ip6_route_output_flags'
> >
> > I see. We can probably use nf_route() instead.
> >
> > Or if needed, use struct nf_ipv6_ops for this.
> >
> > if (v6ops)
> > ret = v6ops->route_xyz(...);
> >
> > @Alin: Would you send us a patch to do so to fix a3419ce3356cf1f
> > netfilter: nf_conntrack_sip: add sip_external_media logic".
>
> nf_ip6_route(net, &dst, &fl6, false) seems to be appropriate.
> I'll send the patch Monday.
I see the original bug I reported is still there. Can you send that patch
you had planned to do?
Arnd
^ permalink raw reply
* Re: [PATCH bpf-next v6 1/9] bpf: implement getsockopt and setsockopt hooks
From: Stanislav Fomichev @ 2019-06-18 18:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Stanislav Fomichev, netdev, bpf, davem, ast, daniel, Martin Lau
In-Reply-To: <20190618164913.GI9636@mini-arch>
On 06/18, Stanislav Fomichev wrote:
> On 06/18, Alexei Starovoitov wrote:
> > On Mon, Jun 17, 2019 at 11:01:01AM -0700, Stanislav Fomichev wrote:
> > > Implement new BPF_PROG_TYPE_CGROUP_SOCKOPT program type and
> > > BPF_CGROUP_{G,S}ETSOCKOPT cgroup hooks.
> > >
> > > BPF_CGROUP_SETSOCKOPT get a read-only view of the setsockopt arguments.
> > > BPF_CGROUP_GETSOCKOPT can modify the supplied buffer.
> > > Both of them reuse existing PTR_TO_PACKET{,_END} infrastructure.
> > >
> > > The buffer memory is pre-allocated (because I don't think there is
> > > a precedent for working with __user memory from bpf). This might be
> > > slow to do for each {s,g}etsockopt call, that's why I've added
> > > __cgroup_bpf_prog_array_is_empty that exits early if there is nothing
> > > attached to a cgroup. Note, however, that there is a race between
> > > __cgroup_bpf_prog_array_is_empty and BPF_PROG_RUN_ARRAY where cgroup
> > > program layout might have changed; this should not be a problem
> > > because in general there is a race between multiple calls to
> > > {s,g}etsocktop and user adding/removing bpf progs from a cgroup.
> > >
> > > The return code of the BPF program is handled as follows:
> > > * 0: EPERM
> > > * 1: success, execute kernel {s,g}etsockopt path after BPF prog exits
> > > * 2: success, do _not_ execute kernel {s,g}etsockopt path after BPF
> > > prog exits
> > >
> > > Note that if 0 or 2 is returned from BPF program, no further BPF program
> > > in the cgroup hierarchy is executed. This is in contrast with any existing
> > > per-cgroup BPF attach_type.
> >
> > This is drastically different from all other cgroup-bpf progs.
> > I think all programs should be executed regardless of return code.
> > It seems to me that 1 vs 2 difference can be expressed via bpf program logic
> > instead of return code.
> >
> > How about we do what all other cgroup-bpf progs do:
> > "any no is no. all yes is yes"
> > Meaning any ret=0 - EPERM back to user.
> > If all are ret=1 - kernel handles get/set.
> >
> > I think the desire to differentiate 1 vs 2 came from ordering issue
> > on getsockopt.
> > How about for setsockopt all progs run first and then kernel.
> > For getsockopt kernel runs first and then all progs.
> > Then progs will have an ability to overwrite anything the kernel returns.
> Good idea, makes sense. For getsockopt we'd also need to pass the return
> value of the kernel getsockopt to let bpf programs override it, but seems
> doable. Let me play with it a bit; I'll send another version if nothing
> major comes up.
>
> Thanks for another round of review!
One clarification: we'd still probably need to have 3 return codes for
setsockopt:
* any 0 - EPERM
* all 1 - continue with the kernel path (i.e. apply this sockopt as is)
* any 2 - return after all BPF hooks are executed (bypass kernel)
(any 0 trumps any 2 -> EPERM)
The context is readonly for setsockopt, so it shouldn't be an issue.
Let me know if you have better idea how to handle that.
^ permalink raw reply
* Re: [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port
From: Florian Fainelli @ 2019-06-18 18:10 UTC (permalink / raw)
To: Benedikt Spranger, netdev
In-Reply-To: <20190618175712.71148-2-b.spranger@linutronix.de>
On 6/18/19 10:57 AM, Benedikt Spranger wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
>
> When enabling Broadcom tags on earlier devices such as BCM53125, we need
> to also enable Managed mode, as well as configure which port is going to
> be the IMP port. If we did not do that, the switch would just pass
> through the Broadcom tags on the wire and not act on them. We need to
> have bcm_sf2 stop overwriting the SWMODE register and let b53 deal with
> that now.
>
> Fixes: 7edc58d614d4 ("net: dsa: b53: Turn on Broadcom tags")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
This patch was part of a bigger series that you don't submit, so you are
simply not just potentially breaking all users of b53, but you are also
breaking bcm_sf2 which you probably don't have access to for testing,
that's unfortunate. More on the cover letter.
--
Florian
^ permalink raw reply
* RE: [PATCH 0/6] net: macb patch set cover letter
From: Parshuram Raju Thombare @ 2019-06-18 18:12 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, netdev@vger.kernel.org,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190617150451.GG25211@lunn.ch>
As this change doesn't affect other users I thought it is safe to do it
from PCI wrapper driver. But yes I agree that right way is to do it in PHY driver.
I would like to drop this patch, please ignore only this patch from series.
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Monday, June 17, 2019 8:35 PM
>To: Parshuram Raju Thombare <pthombar@cadence.com>
>Cc: nicolas.ferre@microchip.com; davem@davemloft.net; f.fainelli@gmail.com;
>netdev@vger.kernel.org; hkallweit1@gmail.com; linux-kernel@vger.kernel.org;
>Rafal Ciepiela <rafalc@cadence.com>; Anil Joy Varughese
><aniljoy@cadence.com>; Piotr Sroka <piotrs@cadence.com>
>Subject: Re: [PATCH 0/6] net: macb patch set cover letter
>
>EXTERNAL MAIL
>
>
>> 3. 003-net-macb-add-PHY-configuration-in-MACB-PCI-wrapper.patch
>> This patch is to configure TI PHY DP83867 in SGMII mode from
>> our MAC PCI wrapper driver.
>> With this change there is no need of PHY driver and dp83867
>> module must be disabled. Users wanting to setup DP83867 PHY
>> in SGMII mode can disable dp83867.ko driver, else dp83867.ko
>> overwrite this configuration and PHY is setup as per dp83867.ko.
>
>This sounds very wrong. Why not make the dp83867 driver support SGMII?
>
> Andrew
Regards,
Parshuram Thombare
^ permalink raw reply
* [PATCH bpf-next] samples: bpf: Remove bpf_debug macro in favor of bpf_printk
From: Michal Rostecki @ 2019-06-18 18:13 UTC (permalink / raw)
Cc: Michal Rostecki, Doug Ledford, Jason Gunthorpe,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, linux-rdma, netdev, bpf, linux-kernel
ibumad example was implementing the bpf_debug macro which is exactly the
same as the bpf_printk macro available in bpf_helpers.h. This change
makes use of bpf_printk instead of bpf_debug.
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
samples/bpf/ibumad_kern.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/samples/bpf/ibumad_kern.c b/samples/bpf/ibumad_kern.c
index 38b2b3f22049..f281df7e0089 100644
--- a/samples/bpf/ibumad_kern.c
+++ b/samples/bpf/ibumad_kern.c
@@ -31,15 +31,9 @@ struct bpf_map_def SEC("maps") write_count = {
};
#undef DEBUG
-#ifdef DEBUG
-#define bpf_debug(fmt, ...) \
-({ \
- char ____fmt[] = fmt; \
- bpf_trace_printk(____fmt, sizeof(____fmt), \
- ##__VA_ARGS__); \
-})
-#else
-#define bpf_debug(fmt, ...)
+#ifndef DEBUG
+#undef bpf_printk
+#define bpf_printk(fmt, ...)
#endif
/* Taken from the current format defined in
@@ -86,7 +80,7 @@ int on_ib_umad_read_recv(struct ib_umad_rw_args *ctx)
u64 zero = 0, *val;
u8 class = ctx->mgmt_class;
- bpf_debug("ib_umad read recv : class 0x%x\n", class);
+ bpf_printk("ib_umad read recv : class 0x%x\n", class);
val = bpf_map_lookup_elem(&read_count, &class);
if (!val) {
@@ -106,7 +100,7 @@ int on_ib_umad_read_send(struct ib_umad_rw_args *ctx)
u64 zero = 0, *val;
u8 class = ctx->mgmt_class;
- bpf_debug("ib_umad read send : class 0x%x\n", class);
+ bpf_printk("ib_umad read send : class 0x%x\n", class);
val = bpf_map_lookup_elem(&read_count, &class);
if (!val) {
@@ -126,7 +120,7 @@ int on_ib_umad_write(struct ib_umad_rw_args *ctx)
u64 zero = 0, *val;
u8 class = ctx->mgmt_class;
- bpf_debug("ib_umad write : class 0x%x\n", class);
+ bpf_printk("ib_umad write : class 0x%x\n", class);
val = bpf_map_lookup_elem(&write_count, &class);
if (!val) {
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] netfilter: nf_conntrack_sip: fix IPV6 dependency
From: Arnd Bergmann @ 2019-06-18 18:13 UTC (permalink / raw)
To: Alin Năstac
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Máté Eckl, Fernando Fernandez Mancera,
netfilter-devel, coreteam, Networking, Linux Kernel Mailing List
In-Reply-To: <CAK8P3a2Go0=9wMfOvdw+GGRNqNZ9c2TJy=jN6dB1VR3K8qz-rg@mail.gmail.com>
On Tue, Jun 18, 2019 at 8:09 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Mar 8, 2019 at 5:23 PM Alin Năstac <alin.nastac@gmail.com> wrote:
> > On Fri, Mar 8, 2019 at 5:04 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Mon, Mar 04, 2019 at 09:40:12PM +0100, Arnd Bergmann wrote:
> > > > With CONFIG_IPV6=m and CONFIG_NF_CONNTRACK_SIP=y, we now get a link failure:
> > > >
> > > > net/netfilter/nf_conntrack_sip.o: In function `process_sdp':
> > > > nf_conntrack_sip.c:(.text+0x4344): undefined reference to `ip6_route_output_flags'
> > >
> > > I see. We can probably use nf_route() instead.
> > >
> > > Or if needed, use struct nf_ipv6_ops for this.
> > >
> > > if (v6ops)
> > > ret = v6ops->route_xyz(...);
> > >
> > > @Alin: Would you send us a patch to do so to fix a3419ce3356cf1f
> > > netfilter: nf_conntrack_sip: add sip_external_media logic".
> >
> > nf_ip6_route(net, &dst, &fl6, false) seems to be appropriate.
> > I'll send the patch Monday.
>
> I see the original bug I reported is still there. Can you send that patch
> you had planned to do?
Hmm, I think I mixed up this patch with a different one that is missing.
Please ignore my reply above.
Arnd
^ permalink raw reply
* RE: [PATCH 0/6] net: macb patch set cover letter
From: Parshuram Raju Thombare @ 2019-06-18 18:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, netdev@vger.kernel.org,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190617150816.GH25211@lunn.ch>
Speed limitation of emulated PHY was because there was existing code
trying to register/emulate PHY for fixed mode.
By removing code to register emulated PHY for fixed mode I am able
to get fixed 10G link up and running.
I will send next version of patch.
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Monday, June 17, 2019 8:38 PM
>To: Parshuram Raju Thombare <pthombar@cadence.com>
>Cc: nicolas.ferre@microchip.com; davem@davemloft.net; f.fainelli@gmail.com;
>netdev@vger.kernel.org; hkallweit1@gmail.com; linux-kernel@vger.kernel.org;
>Rafal Ciepiela <rafalc@cadence.com>; Anil Joy Varughese
><aniljoy@cadence.com>; Piotr Sroka <piotrs@cadence.com>
>Subject: Re: [PATCH 0/6] net: macb patch set cover letter
>
>EXTERNAL MAIL
>
>
>> 5. 0005-net-macb-add-support-for-high-speed-interface
>> This patch add support for 10G USXGMII PCS in fixed mode.
>> Since emulated PHY used in fixed mode doesn't seems to
>> support anything above 1G, additional parameter is used outside
>> "fixed-link" node for selecting speed and "fixed-link"
>> node speed is still set at 1G.
>
>PHYLINK does support higher speeds for fixed-link.
>
> Andrew
Regards,
Parshuram Thombare
^ permalink raw reply
* Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
From: Florian Fainelli @ 2019-06-18 18:16 UTC (permalink / raw)
To: Benedikt Spranger, netdev, Sebastian Andrzej Siewior,
Kurt Kanzenbach
In-Reply-To: <20190618175712.71148-1-b.spranger@linutronix.de>
On 6/18/19 10:57 AM, Benedikt Spranger wrote:
> Hi,
>
> while puting a Banana Pi R1 board into operation I faced network hickups
> and get into serious trouble from my coworkers:
>
> Banana Pi network setup:
> PC (eth1) <--> BPi R1 (wan) / BPi R1 (lan4) <--> DUT (eth0)
> 10.0.32.1 10.0.32.2 172.16.0.1 172.16.0.2
> ---8<---
> #! /bin/bash
>
> # create VLANs
> ip link add link eth0 name eth0.1 type vlan id 1
> ip link add link eth0 name eth0.100 type vlan id 100
>
> # create bridges
> ip link add br0 type bridge
> ip link set dev br0 type bridge vlan_filtering 1
>
> ip link add br1 type bridge
> ip link set dev br1 type bridge vlan_filtering 1
>
> # add interfaces to bridges
> ip link set lan1 master br0
> ip link set lan2 master br0
> ip link set lan3 master br0
> ip link set lan4 master br0
> ip link set eth0.100 master br0
>
> ip link set wan master br1
>
> # adjust tagging
> bridge vlan add dev lan1 vid 100 pvid untagged
> bridge vlan add dev lan2 vid 100 pvid untagged
> bridge vlan add dev lan3 vid 100 pvid untagged
> bridge vlan add dev lan4 vid 100 pvid untagged
>
> bridge vlan del dev lan1 vid 1
> bridge vlan del dev lan2 vid 1
> bridge vlan del dev lan3 vid 1
> bridge vlan del dev lan4 vid 1
>
> # set IPs
> ip addr add 10.0.32.2/24 dev eth0.1
> ip addr add 172.16.0.1/24 dev br0
>
> # up and run
> ip link set eth0 up
> ip link set eth0.1 up
> ip link set eth0.100 up
>
> ip link set br0 up
> ip link set br1 up
>
> ip link set wan up
>
> ip link set lan1 up
> ip link set lan2 up
> ip link set lan3 up
> ip link set lan4 up
> ---8<---
>
> While trying to ping a non existing host 10.0.32.111 result in
> doublication of ARP broadcasts:
>
> On the BPi R1:
> # tshark -i br0
> 1 8.157471671 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
> Tell 10.0.32.1
> 2 9.167563213 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
> Tell 10.0.32.1
> 3 10.191703047 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 4 11.215905797 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 5 12.243932964 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 6 13.264033298 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>
> # tshark -i wan0
> 1 8.157421295 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
> Tell 10.0.32.1
> 2 9.167510087 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111?
> Tell 10.0.32.1
> 3 10.191649213 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 4 11.215856838 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 5 12.243881922 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 6 13.263981506 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>
> On the PC:
> # tshark -i eth1
> 116 272.660717059 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
> 117 272.661062292 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
> 118 273.670690338 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
> 119 273.671058605 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
> 120 274.694720511 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
> 121 274.695250723 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
> 122 275.718813538 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
> 123 275.719285852 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
> 124 276.746729795 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
> 125 276.747236341 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
> 126 277.766722429 DavicomS_43:18:fc → Broadcast ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
> 127 277.767233098 DavicomS_43:18:fc → Broadcast ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
>
> Choosing between network disturbance by mirrored broadcast packages (and angry
> coworkers) or lack of multicast, I choose the later.
How is that a problem for other machines? Does that lead to some kind of
broadcast storm because there are machines that keep trying to respond
to ARP solicitations?
The few aspects that bother me, not in any particular order, are that:
- you might be able to not change anything and just get away with the
one line patch below that sets skb->offload_fwd_mark to 1 to indicate to
the bridge, not to bother with sending a copy of the packet, since the
HW took care of that already
- the patch from me that you included was part of a larger series that
also addressed multicast while in management mode, such we would not
have to chose like you did, have you tested it, did it somehow not work?
- you have not copied the DSA maintainers on all of the patches, so you
get a C grade for your patch submission
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 9c3114179690..9169b63a89e3 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -140,6 +140,8 @@ static struct sk_buff *brcm_tag_rcv_ll(struct
sk_buff *skb,
/* Remove Broadcom tag and update checksum */
skb_pull_rcsum(skb, BRCM_TAG_LEN);
+ skb->offload_fwd_mark = 1;
+
return skb;
}
#endif
--
Florian
^ permalink raw reply related
* Re: [PATCH 1/2] net: fastopen: make key handling more robust against future changes
From: Eric Biggers @ 2019-06-18 18:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ard Biesheuvel, netdev,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu,
David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jason Baron,
Christoph Paasch, David Laight, Yuchung Cheng
In-Reply-To: <CANn89i+X7YQ6DueDQAusA+1S5Kmo75OwzO+eYRZe_nR8=YWjuQ@mail.gmail.com>
On Tue, Jun 18, 2019 at 02:53:05AM -0700, Eric Dumazet wrote:
> On Tue, Jun 18, 2019 at 2:41 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > On Tue, 18 Jun 2019 at 11:39, Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Tue, Jun 18, 2019 at 2:32 AM Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org> wrote:
> > > >
> > > > Some changes to the TCP fastopen code to make it more robust
> > > > against future changes in the choice of key/cookie size, etc.
> > > >
> > > > - Instead of keeping the SipHash key in an untyped u8[] buffer
> > > > and casting it to the right type upon use, use the correct
> > > > siphash_key_t type directly. This ensures that the key will
> > > > appear at the correct alignment if we ever change the way
> > > > these data structures are allocated. (Currently, they are
> > > > only allocated via kmalloc so they always appear at the
> > > > correct alignment)
> > > >
> > > > - Use DIV_ROUND_UP when sizing the u64[] array to hold the
> > > > cookie, so it is always of sufficient size, even when
> > > > TCP_FASTOPEN_COOKIE_MAX is no longer a multiple of 8.
> > > >
> > > > - Add a key length check to tcp_fastopen_reset_cipher(). No
> > > > callers exist currently that fail this check (they all pass
> > > > compile constant values that equal TCP_FASTOPEN_KEY_LENGTH),
> > > > but future changes might create problems, e.g., by leaving part
> > > > of the key uninitialized, or overflowing the key buffers.
> > > >
> > > > Note that none of these are functional changes wrt the current
> > > > state of the code.
> > > >
> > > ...
> > >
> > > > - memcpy(ctx->key[0], primary_key, len);
> > > > + if (unlikely(len != TCP_FASTOPEN_KEY_LENGTH)) {
> > > > + pr_err("TCP: TFO key length %u invalid\n", len);
> > > > + err = -EINVAL;
> > > > + goto out;
> > > > + }
> > >
> > >
> > > Why a pr_err() is there ?
> > >
> > > Can unpriv users flood the syslog ?
> >
> > They can if they could do so before: there was a call to
> > crypto_cipher_setkey() in the original pre-SipHash code which would
> > also result in a pr_err() on an invalid key length. That call got
> > removed along with the AES cipher handling, and this basically
> > reinstates it, as suggested by EricB.
>
> This tcp_fastopen_reset_cipher() function is internal to TCP stack, all callers
> always pass the correct length.
>
> We could add checks all over the place, and end up having a TCP stack
> full of defensive
> checks and 10,000 additional lines of code :/
>
> I would prefer not reinstating this.
The length parameter makes no sense if it's not checked, though. Either it
should exist and be checked, or it should be removed and the length should be
implicitly TCP_FASTOPEN_KEY_LENGTH.
- Eric
^ permalink raw reply
* RE: [PATCH 5/6] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-18 18:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, netdev@vger.kernel.org,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190617151927.GI25211@lunn.ch>
>
>> switch (state->interface) {
>> + case PHY_INTERFACE_MODE_NA:
>
>I would not list PHY_INTERFACE_MODE_NA here.
>
This was to experiment in band mode with sfp.
phylink_sfp_module_insert call phylink_validate with interface set to PHY_INTERFACE_MODE_NA
, if it is not listed in validate method supported bitmask will be empty.
But anyway since I am configuring fixed mode, removing this case.
>> + case PHY_INTERFACE_MODE_USXGMII:
>> + case PHY_INTERFACE_MODE_10GKR:
>> + if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
>> + phylink_set(mask, 10000baseCR_Full);
>> + phylink_set(mask, 10000baseER_Full);
>> + phylink_set(mask, 10000baseKR_Full);
>> + phylink_set(mask, 10000baseLR_Full);
>> + phylink_set(mask, 10000baseLRM_Full);
>> + phylink_set(mask, 10000baseSR_Full);
>> + phylink_set(mask, 10000baseT_Full);
>> + phylink_set(mask, 5000baseT_Full);
>> + phylink_set(mask, 2500baseX_Full);
>> + phylink_set(mask, 1000baseX_Full);
>> + }
>> + /* Fall-through */
>> case PHY_INTERFACE_MODE_SGMII:
>> if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>> phylink_set(mask, 2500baseT_Full); @@ -594,17 +639,55
>@@ static
>> void gem_mac_config(struct phylink_config *pl_config, unsigned int mode,
>> reg |= MACB_BIT(FD);
>> macb_or_gem_writel(bp, NCFGR, reg);
>>
>> - if (state->speed == SPEED_2500) {
>> - gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> - gem_readl(bp, NCFGR));
>> - gem_writel(bp, NCR, GEM_BIT(TWO_PT_FIVE_GIG) |
>> - gem_readl(bp, NCR));
>> - } else if (state->speed == SPEED_1000) {
>> - gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> - gem_readl(bp, NCFGR));
>> - } else if (state->speed == SPEED_100) {
>> - macb_writel(bp, NCFGR, MACB_BIT(SPD) |
>> - macb_readl(bp, NCFGR));
>> + if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
>> + u32 speed;
>> +
>> + switch (state->speed) {
>> + case SPEED_10000:
>> + if (bp->serdes_rate ==
>> + MACB_SERDES_RATE_10_PT_3125Gbps) {
>> + speed = HS_MAC_SPEED_10000M;
>> + } else {
>> + netdev_warn(netdev,
>> + "10G not supported by HW");
>> + netdev_warn(netdev, "Setting speed to
>1G");
>> + speed = HS_MAC_SPEED_1000M;
>> + }
>> + break;
>> + case SPEED_5000:
>> + speed = HS_MAC_SPEED_5000M;
>> + break;
>> + case SPEED_2500:
>> + speed = HS_MAC_SPEED_2500M;
>> + break;
>> + case SPEED_1000:
>> + speed = HS_MAC_SPEED_1000M;
>> + break;
>> + default:
>> + case SPEED_100:
>> + speed = HS_MAC_SPEED_100M;
>> + break;
>> + }
>> +
>> + gem_writel(bp, HS_MAC_CONFIG,
>> + GEM_BFINS(HS_MAC_SPEED, speed,
>> + gem_readl(bp, HS_MAC_CONFIG)));
>> + gem_writel(bp, USX_CONTROL,
>> + GEM_BFINS(USX_CTRL_SPEED, speed,
>> + gem_readl(bp, USX_CONTROL)));
>> + } else {
>> + if (state->speed == SPEED_2500) {
>> + gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> + gem_readl(bp, NCFGR));
>> + gem_writel(bp, NCR,
>GEM_BIT(TWO_PT_FIVE_GIG) |
>> + gem_readl(bp, NCR));
>> + } else if (state->speed == SPEED_1000) {
>> + gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> + gem_readl(bp, NCFGR));
>> + } else if (state->speed == SPEED_100) {
>> + macb_writel(bp, NCFGR, MACB_BIT(SPD) |
>> + macb_readl(bp, NCFGR));
>> + }
>
>Maybe split this up into two helper functions?
Ok
>
> Andrew
Regards,
Parshuram Thombare
^ permalink raw reply
* Re: [PATCH 1/2] net: fastopen: make key handling more robust against future changes
From: Eric Dumazet @ 2019-06-18 18:19 UTC (permalink / raw)
To: Eric Biggers
Cc: Ard Biesheuvel, netdev,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu,
David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jason Baron,
Christoph Paasch, David Laight, Yuchung Cheng
In-Reply-To: <20190618181804.GJ184520@gmail.com>
On Tue, Jun 18, 2019 at 11:18 AM Eric Biggers <ebiggers@kernel.org> wrote:
> The length parameter makes no sense if it's not checked, though. Either it
> should exist and be checked, or it should be removed and the length should be
> implicitly TCP_FASTOPEN_KEY_LENGTH.
Sure, please send a patch.
^ permalink raw reply
* RE: [PATCH 6/6] net: macb: parameter added to cadence ethernet controller DT binding
From: Parshuram Raju Thombare @ 2019-06-18 18:19 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, netdev@vger.kernel.org,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190617152118.GJ25211@lunn.ch>
>On Sun, Jun 16, 2019 at 12:49:39AM +0100, Parshuram Thombare wrote:
>> New parameters added to Cadence ethernet controller DT binding for
>> USXGMII interface.
>>
>> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
>> ---
>> Documentation/devicetree/bindings/net/macb.txt | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/macb.txt
>> b/Documentation/devicetree/bindings/net/macb.txt
>> index 9c5e94482b5f..cd79ec9dddfb 100644
>> --- a/Documentation/devicetree/bindings/net/macb.txt
>> +++ b/Documentation/devicetree/bindings/net/macb.txt
>> @@ -25,6 +25,10 @@ Required properties:
>> Optional elements: 'rx_clk' applies to cdns,zynqmp-gem
>> Optional elements: 'tsu_clk'
>> - clocks: Phandles to input clocks.
>> +- serdes-rate External serdes rate.Mandatory for USXGMII mode.
>> + 0 - 5G
>> + 1 - 10G
>
Ok
>Please use the values 5 and 10, not 0 and 1. This also needs a vendor prefix.
>
> Andrew
Regards,
Parshuram Thombare
^ permalink raw reply
* RE: [PATCH 1/6] net: macb: add phylink support
From: Parshuram Raju Thombare @ 2019-06-18 18:22 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, netdev@vger.kernel.org,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190617174242.GL17551@lunn.ch>
>> @@ -4217,8 +4257,8 @@ static int macb_probe(struct platform_device *pdev)
>>
>> tasklet_init(&bp->hresp_err_tasklet, macb_hresp_error_task,
>> (unsigned long)bp);
>> -
>> - phy_attached_info(phydev);
>> + if (dev->phydev)
>> + phy_attached_info(dev->phydev);
>
>When can this happen? I don't see anything assigning to dev->phydev.
This is for non sfp (MDIO) based PHY. It is set in phy_attach_direct (phylink_connect_phy -> __ phylink_connect_phy -> phy_attach_direct)
>
> Andrew
Regards,
Parshuram Thombare
^ permalink raw reply
* Re: [PATCH 2/2] net: fastopen: use endianness agnostic representation of the cookie
From: Eric Biggers @ 2019-06-18 18:22 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: netdev, linux-crypto, herbert, edumazet, davem, kuznet, yoshfuji,
jbaron, cpaasch, David.Laight, ycheng
In-Reply-To: <20190618093207.13436-3-ard.biesheuvel@linaro.org>
On Tue, Jun 18, 2019 at 11:32:07AM +0200, Ard Biesheuvel wrote:
> Use an explicit little endian representation of the fastopen
> cookie, so that the value no longer depends on the endianness
> of the system. This fixes a theoretical issue only, since
> fastopen keys are unlikely to be shared across load balancing
> server farms that are mixed in endiannes, but it might pop up
> in validation/selftests as well, so let's just settle on little
> endian across the board.
>
> Note that this change only affects big endian systems.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> include/linux/tcp.h | 2 +-
> net/ipv4/tcp_fastopen.c | 16 ++++++++--------
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
What about the TCP_FASTOPEN_KEY option for setsockopt and getsockopt? Those
APIs treat the key as an array of bytes (let's say it's little endian), so
doesn't it need to be converted to/from the CPU endianness of siphash_key_t?
- Eric
^ 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