* Re: [PATCH] r8152: Set memory to all 0xFFs on failed reg reads
From: David Miller @ 2019-08-26 2:53 UTC (permalink / raw)
To: pmalani; +Cc: hayeswang, grundler, netdev
In-Reply-To: <20190824083619.69139-1-pmalani@chromium.org>
From: Prashant Malani <pmalani@chromium.org>
Date: Sat, 24 Aug 2019 01:36:19 -0700
> get_registers() blindly copies the memory written to by the
> usb_control_msg() call even if the underlying urb failed.
>
> This could lead to junk register values being read by the driver, since
> some indirect callers of get_registers() ignore the return values. One
> example is:
> ocp_read_dword() ignores the return value of generic_ocp_read(), which
> calls get_registers().
>
> So, emulate PCI "Master Abort" behavior by setting the buffer to all
> 0xFFs when usb_control_msg() fails.
>
> This patch is copied from the r8152 driver (v2.12.0) published by
> Realtek (www.realtek.com).
>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
Applied.
^ permalink raw reply
* Re: [PATCH v2 net-next] cirrus: cs89x0: remove set but not used variable 'lp'
From: David Miller @ 2019-08-26 2:50 UTC (permalink / raw)
To: yuehaibing; +Cc: netdev, kernel-janitors, hulkci
In-Reply-To: <20190826024915.67642-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 26 Aug 2019 02:49:15 +0000
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/cirrus/cs89x0.c: In function 'cs89x0_platform_probe':
> drivers/net/ethernet/cirrus/cs89x0.c:1847:20: warning:
> variable 'lp' set but not used [-Wunused-but-set-variable]
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: 6751edeb8700 ("cirrus: cs89x0: Use managed interfaces")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: fix skb use after free in netpoll_send_skb_on_dev
From: David Miller @ 2019-08-26 2:48 UTC (permalink / raw)
To: loyou85
Cc: edumazet, dsterba, dbanerje, fw, davej, tglx, matwey,
sakari.ailus, netdev, linux-kernel, xiaojunzhao141
In-Reply-To: <1566577920-20956-1-git-send-email-loyou85@gmail.com>
From: Feng Sun <loyou85@gmail.com>
Date: Sat, 24 Aug 2019 00:32:00 +0800
> After commit baeababb5b85d5c4e6c917efe2a1504179438d3b
> ("tun: return NET_XMIT_DROP for dropped packets"),
> when tun_net_xmit drop packets, it will free skb and return NET_XMIT_DROP,
> netpoll_send_skb_on_dev will run into two use after free cases:
I don't know what to do here.
Really, the intention of the design is that the only valid
->ndo_start_xmit() values are those with macro names fitting the
pattern NETDEV_TX_*, which means only NETDEV_TX_OK and NETDEV_TX_BUSY
are valid.
NET_XMIT_* values are for qdisc ->enqueue() methods.
Note, particularly, that when ->ndo_start_xmit() values are propagated
through ->enqueue() calls they get masked out with NET_XMIT_MASK.
However, I see that most of the code doing enqueueing and invocation
of ->ndo_start_xmit() use the dev_xmit_complete() helper to check this
condition.
So probably that is what netpoll should be using as well.
^ permalink raw reply
* Re: [PATCH] bridge:fragmented packets dropped by bridge
From: Rundong Ge @ 2019-08-26 2:45 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: davem, kuznet, yoshfuji, netdev, Pablo Neira Ayuso, kadlec,
Florian Westphal, Roopa Prabhu, netfilter-devel, coreteam, bridge,
linux-kernel
In-Reply-To: <1dc87e69-628b-fd04-619a-8dbe5bdfa108@cumulusnetworks.com>
On Tue, Jul 30, 2019 at 8:41 PM Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
>
> On 30/07/2019 15:25, Rundong Ge wrote:
> > Given following setup:
> > -modprobe br_netfilter
> > -echo '1' > /proc/sys/net/bridge/bridge-nf-call-iptables
> > -brctl addbr br0
> > -brctl addif br0 enp2s0
> > -brctl addif br0 enp3s0
> > -brctl addif br0 enp6s0
> > -ifconfig enp2s0 mtu 1300
> > -ifconfig enp3s0 mtu 1500
> > -ifconfig enp6s0 mtu 1500
> > -ifconfig br0 up
> >
> > multi-port
> > mtu1500 - mtu1500|bridge|1500 - mtu1500
> > A | B
> > mtu1300
> >
> > With netfilter defragmentation/conntrack enabled, fragmented
> > packets from A will be defragmented in prerouting, and refragmented
> > at postrouting.
> > But in this scenario the bridge found the frag_max_size(1500) is
> > larger than the dst mtu stored in the fake_rtable whitch is
> > always equal to the bridge's mtu 1300, then packets will be dopped.
> >
> > This modifies ip_skb_dst_mtu to use the out dev's mtu instead
> > of bridge's mtu in bridge refragment.
> >
> > Signed-off-by: Rundong Ge <rdong.ge@gmail.com>
> > ---
> > include/net/ip.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/include/net/ip.h b/include/net/ip.h
> > index 29d89de..0512de3 100644
> > --- a/include/net/ip.h
> > +++ b/include/net/ip.h
> > @@ -450,6 +450,8 @@ static inline unsigned int ip_dst_mtu_maybe_forward(const struct dst_entry *dst,
> > static inline unsigned int ip_skb_dst_mtu(struct sock *sk,
> > const struct sk_buff *skb)
> > {
> > + if ((skb_dst(skb)->flags & DST_FAKE_RTABLE) && skb->dev)
> > + return min(skb->dev->mtu, IP_MAX_MTU);
> > if (!sk || !sk_fullsock(sk) || ip_sk_use_pmtu(sk)) {
> > bool forwarding = IPCB(skb)->flags & IPSKB_FORWARDED;
> >
> >
>
> I don't think this is correct, there's a reason why the bridge chooses the smallest
> possible MTU out of its members and this is simply a hack to circumvent it.
> If you really like to do so just set the bridge MTU manually, we've added support
> so it won't change automatically to the smallest, but then how do you pass packets
> 1500 -> 1300 in this setup ?
>
> You're talking about the frag_size check in br_nf_ip_fragment(), right ?
>
Hi Nikolay
My setup may not be common. And may I know if there is any reason to
use output port's MTU
to do the re-fragment check but then use the bridge's MTU to do the re-fragment?
Is it the expected behavior that the bridge's MTU will affect the
FORWARD traffic re-fragment,
because I used to think the bridge's MTU will only effect the OUTPUT
traffic sent from "br0".
And the modification in this patch will replace the MTU in the
fake_rtable which is only
used in the FORWARD re-fragment and won't affect the local traffic from "br0".
TKS
Raydodn
^ permalink raw reply
* [PATCH v2 net-next] cirrus: cs89x0: remove set but not used variable 'lp'
From: YueHaibing @ 2019-08-26 2:49 UTC (permalink / raw)
To: David S . Miller, YueHaibing; +Cc: netdev, kernel-janitors, Hulk Robot
In-Reply-To: <20190822063517.71231-1-yuehaibing@huawei.com>
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/ethernet/cirrus/cs89x0.c: In function 'cs89x0_platform_probe':
drivers/net/ethernet/cirrus/cs89x0.c:1847:20: warning:
variable 'lp' set but not used [-Wunused-but-set-variable]
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 6751edeb8700 ("cirrus: cs89x0: Use managed interfaces")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v2: add Fixes tag
---
drivers/net/ethernet/cirrus/cs89x0.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/cirrus/cs89x0.c b/drivers/net/ethernet/cirrus/cs89x0.c
index 2d30972df06b..c9aebcde403a 100644
--- a/drivers/net/ethernet/cirrus/cs89x0.c
+++ b/drivers/net/ethernet/cirrus/cs89x0.c
@@ -1844,15 +1844,12 @@ cleanup_module(void)
static int __init cs89x0_platform_probe(struct platform_device *pdev)
{
struct net_device *dev = alloc_etherdev(sizeof(struct net_local));
- struct net_local *lp;
void __iomem *virt_addr;
int err;
if (!dev)
return -ENOMEM;
- lp = netdev_priv(dev);
-
dev->irq = platform_get_irq(pdev, 0);
if (dev->irq <= 0) {
dev_warn(&dev->dev, "interrupt resource missing\n");
^ permalink raw reply related
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2019-08-26 2:27 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Heiner Kallweit
[-- Attachment #1: Type: text/plain, Size: 851 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
drivers/net/ethernet/realtek/r8169_main.c
between commit:
345b93265b3a ("Revert "r8169: remove not needed call to dma_sync_single_for_device"")
from the net tree and commit:
fcd4e60885af ("r8169: improve rtl_rx")
d4ed7463d02a ("r8169: fix DMA issue on MIPS platform")
from the net-next tree.
I fixed it up (the latter seems to do the same as the net tree patch) and
can carry the fix as necessary. This is now fixed as far as linux-next
is concerned, but any non trivial conflicts should be mentioned to your
upstream maintainer when your tree is submitted for merging. You may
also want to consider cooperating with the maintainer of the conflicting
tree to minimise any particularly complex conflicts.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* RE: [PATCH net-next v4 0/2] r8152: save EEE
From: Hayes Wang @ 2019-08-26 2:25 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org
In-Reply-To: <20190823143722.GE21295@lunn.ch>
Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Friday, August 23, 2019 10:37 PM
[...]
> Hi Hayes
>
> That was 3 revisions of the patches in less than 30 minutes. Slow
> down, take your time, review your work yourself before posting it,
> etc.
>
> Aim for no more than one revision, posted to the list, per day.
Sorry. I would note that.
Best Regards,
Hayes
^ permalink raw reply
* RE: [PATCH] r8152: Set memory to all 0xFFs on failed reg reads
From: Hayes Wang @ 2019-08-26 2:25 UTC (permalink / raw)
To: Prashant Malani, davem@davemloft.net
Cc: grundler@chromium.org, netdev@vger.kernel.org, nic_swsd
In-Reply-To: <20190824083619.69139-1-pmalani@chromium.org>
Prashant Malani [mailto:pmalani@chromium.org]
> Sent: Saturday, August 24, 2019 4:36 PM
[...]
> get_registers() blindly copies the memory written to by the
> usb_control_msg() call even if the underlying urb failed.
>
> This could lead to junk register values being read by the driver, since
> some indirect callers of get_registers() ignore the return values. One
> example is:
> ocp_read_dword() ignores the return value of generic_ocp_read(), which
> calls get_registers().
>
> So, emulate PCI "Master Abort" behavior by setting the buffer to all
> 0xFFs when usb_control_msg() fails.
>
> This patch is copied from the r8152 driver (v2.12.0) published by
> Realtek (www.realtek.com).
>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
Acked-by: Hayes Wang <hayeswang@realtek.com>
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH v2 -next] net: mediatek: remove set but not used variable 'status'
From: David Miller @ 2019-08-26 2:06 UTC (permalink / raw)
To: maowenan
Cc: nbd, john, sean.wang, nelson.chang, matthias.bgg, kernel-janitors,
netdev, linux-arm-kernel, linux-mediatek, linux-kernel
In-Reply-To: <20190826013118.22720-1-maowenan@huawei.com>
From: Mao Wenan <maowenan@huawei.com>
Date: Mon, 26 Aug 2019 09:31:18 +0800
> Fixes gcc '-Wunused-but-set-variable' warning:
> drivers/net/ethernet/mediatek/mtk_eth_soc.c: In function mtk_handle_irq:
> drivers/net/ethernet/mediatek/mtk_eth_soc.c:1951:6: warning: variable status set but not used [-Wunused-but-set-variable]
>
> Fixes: 296c9120752b ("net: ethernet: mediatek: Add MT7628/88 SoC support")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
Are you sure the register isn't being read in order to make some
hardware side effect happen?
Have you tested this on effected hardware?
I'm not applying this without definitive answers to these questions.
^ permalink raw reply
* [PATCH v2 -next] net: mediatek: remove set but not used variable 'status'
From: Mao Wenan @ 2019-08-26 1:31 UTC (permalink / raw)
To: nbd, john, sean.wang, nelson.chang, davem, matthias.bgg
Cc: kernel-janitors, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel, Mao Wenan
In-Reply-To: <20190824.142158.1506174328495468705.davem@davemloft.net>
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/ethernet/mediatek/mtk_eth_soc.c: In function mtk_handle_irq:
drivers/net/ethernet/mediatek/mtk_eth_soc.c:1951:6: warning: variable status set but not used [-Wunused-but-set-variable]
Fixes: 296c9120752b ("net: ethernet: mediatek: Add MT7628/88 SoC support")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
v2: change format of 'Fixes' tag.
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8ddbb8d..bb7d623 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1948,9 +1948,7 @@ static irqreturn_t mtk_handle_irq_tx(int irq, void *_eth)
static irqreturn_t mtk_handle_irq(int irq, void *_eth)
{
struct mtk_eth *eth = _eth;
- u32 status;
- status = mtk_r32(eth, MTK_PDMA_INT_STATUS);
if (mtk_r32(eth, MTK_PDMA_INT_MASK) & MTK_RX_DONE_INT) {
if (mtk_r32(eth, MTK_PDMA_INT_STATUS) & MTK_RX_DONE_INT)
mtk_handle_irq_rx(irq, _eth);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net 1/2] openvswitch: Properly set L4 keys on "later" IP fragments.
From: David Miller @ 2019-08-25 21:52 UTC (permalink / raw)
To: pshelar; +Cc: jpettit, netdev, joe
In-Reply-To: <CAOrHB_BKd=QKR_ScO+r7qAyZaniEbFur+iup1iXbtiycaFawjw@mail.gmail.com>
From: Pravin Shelar <pshelar@ovn.org>
Date: Sun, 25 Aug 2019 13:40:58 -0700
> On Sun, Aug 25, 2019 at 9:54 AM Pravin Shelar <pshelar@ovn.org> wrote:
>>
>> On Sat, Aug 24, 2019 at 9:58 AM Justin Pettit <jpettit@ovn.org> wrote:
>> >
>> > When IP fragments are reassembled before being sent to conntrack, the
>> > key from the last fragment is used. Unless there are reordering
>> > issues, the last fragment received will not contain the L4 ports, so the
>> > key for the reassembled datagram won't contain them. This patch updates
>> > the key once we have a reassembled datagram.
>> >
>> > Signed-off-by: Justin Pettit <jpettit@ovn.org>
>> > ---
>> > net/openvswitch/conntrack.c | 4 ++++
>> > 1 file changed, 4 insertions(+)
>> >
>> > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
>> > index 848c6eb55064..f40ad2a42086 100644
>> > --- a/net/openvswitch/conntrack.c
>> > +++ b/net/openvswitch/conntrack.c
>> > @@ -524,6 +524,10 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key,
>> > return -EPFNOSUPPORT;
>> > }
>> >
>> > + /* The key extracted from the fragment that completed this datagram
>> > + * likely didn't have an L4 header, so regenerate it. */
>> > + ovs_flow_key_update(skb, key);
>> > +
>> > key->ip.frag = OVS_FRAG_TYPE_NONE;
>> > skb_clear_hash(skb);
>> > skb->ignore_df = 1;
>> > --
>>
>> Looks good to me.
>>
>> Acked-by: Pravin B Shelar <pshelar@ovn.org>
>>
> Actually I am not sure about this change. caller of this function
> (ovs_ct_execute()) does skb-pull and push of L2 header, calling
> ovs_flow_key_update() is not safe here, it expect skb data to point to
> L2 header.
Agreed, also the comment needs to be formatted properly ala:
/* blah
* blah
*/
instead of:
/* blah
* blah */
^ permalink raw reply
* Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout
From: David Miller @ 2019-08-25 21:49 UTC (permalink / raw)
To: yihung.wei; +Cc: netdev, pshelar
In-Reply-To: <1566505070-38748-1-git-send-email-yihung.wei@gmail.com>
From: Yi-Hung Wei <yihung.wei@gmail.com>
Date: Thu, 22 Aug 2019 13:17:50 -0700
> This patch addresses a conntrack cache issue with timeout policy.
> Currently, we do not check if the timeout extension is set properly in the
> cached conntrack entry. Thus, after packet recirculate from conntrack
> action, the timeout policy is not applied properly. This patch fixes the
> aforementioned issue.
>
> Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action")
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] net: dsa: tag_8021q: Future-proof the reserved fields in the custom VID
From: Florian Fainelli @ 2019-08-25 21:46 UTC (permalink / raw)
To: Vladimir Oltean, vivien.didelot, andrew, davem; +Cc: netdev
In-Reply-To: <20190825183212.11426-1-olteanv@gmail.com>
Le 8/25/19 à 11:32 AM, Vladimir Oltean a écrit :
> After witnessing the discussion in https://lkml.org/lkml/2019/8/14/151
> w.r.t. ioctl extensibility, it became clear that such an issue might
> prevent that the 3 RSV bits inside the DSA 802.1Q tag might also suffer
> the same fate and be useless for further extension.
>
> So clearly specify that the reserved bits should currently be
> transmitted as zero and ignored on receive. The DSA tagger already does
> this (and has always did), and is the only known user so far (no
> Wireshark dissection plugin, etc). So there should be no incompatibility
> to speak of.
>
> Fixes: 0471dd429cea ("net: dsa: tag_8021q: Create a stable binary format")
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net] ipv4: mpls: fix mpls_xmit for iptunnel
From: David Miller @ 2019-08-25 21:35 UTC (permalink / raw)
To: alexey.kodanev; +Cc: netdev, dsahern
In-Reply-To: <1566582703-26567-1-git-send-email-alexey.kodanev@oracle.com>
From: Alexey Kodanev <alexey.kodanev@oracle.com>
Date: Fri, 23 Aug 2019 20:51:43 +0300
> When using mpls over gre/gre6 setup, rt->rt_gw4 address is not set, the
> same for rt->rt_gw_family. Therefore, when rt->rt_gw_family is checked
> in mpls_xmit(), neigh_xmit() call is skipped. As a result, such setup
> doesn't work anymore.
>
> This issue was found with LTP mpls03 tests.
>
> Fixes: 1550c171935d ("ipv4: Prepare rtable for IPv6 gateway")
> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
Applied and queued up for v5.2 -stable.
Thanks.
^ permalink raw reply
* Re: [PATCH net] nexthop: Fix nexthop_num_path for blackhole nexthops
From: David Miller @ 2019-08-25 21:32 UTC (permalink / raw)
To: dsahern; +Cc: netdev, sharpd, dsahern
In-Reply-To: <20190825144730.28814-1-dsahern@kernel.org>
From: David Ahern <dsahern@kernel.org>
Date: Sun, 25 Aug 2019 07:47:30 -0700
> From: David Ahern <dsahern@gmail.com>
>
> Donald reported this sequence:
> ip next add id 1 blackhole
> ip next add id 2 blackhole
> ip ro add 1.1.1.1/32 nhid 1
> ip ro add 1.1.1.2/32 nhid 2
>
> would cause a crash. Backtrace is:
...
> fib_dump_info incorrectly has nhs = 0 for blackhole nexthops, so it
> believes the nexthop object is a multipath group (nhs != 1) and ends
> up down the nexthop_mpath_fill_node() path which is wrong for a
> blackhole.
>
> The blackhole check in nexthop_num_path is leftover from early days
> of the blackhole implementation which did not initialize the device.
> In the end the design was simpler (fewer special case checks) to set
> the device to loopback in nh_info, so the check in nexthop_num_path
> should have been removed.
>
> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
> Reported-by: Donald Sharp <sharpd@cumulusnetworks.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH net 1/2] openvswitch: Properly set L4 keys on "later" IP fragments.
From: Pravin Shelar @ 2019-08-25 20:40 UTC (permalink / raw)
To: Justin Pettit; +Cc: Linux Kernel Network Developers, Joe Stringer
In-Reply-To: <CAOrHB_AU1gQ74L5WawyA4THhh=MG8YZhcvkkxnKgRG+5m-436g@mail.gmail.com>
On Sun, Aug 25, 2019 at 9:54 AM Pravin Shelar <pshelar@ovn.org> wrote:
>
> On Sat, Aug 24, 2019 at 9:58 AM Justin Pettit <jpettit@ovn.org> wrote:
> >
> > When IP fragments are reassembled before being sent to conntrack, the
> > key from the last fragment is used. Unless there are reordering
> > issues, the last fragment received will not contain the L4 ports, so the
> > key for the reassembled datagram won't contain them. This patch updates
> > the key once we have a reassembled datagram.
> >
> > Signed-off-by: Justin Pettit <jpettit@ovn.org>
> > ---
> > net/openvswitch/conntrack.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> > index 848c6eb55064..f40ad2a42086 100644
> > --- a/net/openvswitch/conntrack.c
> > +++ b/net/openvswitch/conntrack.c
> > @@ -524,6 +524,10 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key,
> > return -EPFNOSUPPORT;
> > }
> >
> > + /* The key extracted from the fragment that completed this datagram
> > + * likely didn't have an L4 header, so regenerate it. */
> > + ovs_flow_key_update(skb, key);
> > +
> > key->ip.frag = OVS_FRAG_TYPE_NONE;
> > skb_clear_hash(skb);
> > skb->ignore_df = 1;
> > --
>
> Looks good to me.
>
> Acked-by: Pravin B Shelar <pshelar@ovn.org>
>
Actually I am not sure about this change. caller of this function
(ovs_ct_execute()) does skb-pull and push of L2 header, calling
ovs_flow_key_update() is not safe here, it expect skb data to point to
L2 header.
^ permalink raw reply
* [PATCH net-next 2/2] net: dsa: sja1105: Clear VLAN filtering offload netdev feature
From: Vladimir Oltean @ 2019-08-25 19:46 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190825194630.12404-1-olteanv@gmail.com>
The switch barely supports traffic I/O, and it does that by repurposing
VLANs when there is no bridge that is taking control of them.
Letting DSA declare this netdev feature as supported (see
dsa_slave_create) would mean that VLAN sub-interfaces created on sja1105
switch ports will be hardware offloaded. That means that
net/8021q/vlan_core.c would install the VLAN into the filter tables of
the switch, potentially interfering with the tag_8021q VLANs.
We need to prevent that from happening and not let the 8021q core
offload VLANs to the switch hardware tables. In vlan_filtering=0 modes
of operation, the switch ports can pass through VLAN-tagged frames with
no problem.
Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index df976b259e43..d8cff0107ec4 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1728,6 +1728,21 @@ static void sja1105_teardown(struct dsa_switch *ds)
sja1105_static_config_free(&priv->static_config);
}
+static int sja1105_port_enable(struct dsa_switch *ds, int port,
+ struct phy_device *phy)
+{
+ struct net_device *slave;
+
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
+ slave = ds->ports[port].slave;
+
+ slave->features &= ~NETIF_F_HW_VLAN_CTAG_FILTER;
+
+ return 0;
+}
+
static int sja1105_mgmt_xmit(struct dsa_switch *ds, int port, int slot,
struct sk_buff *skb, bool takets)
{
@@ -2049,6 +2064,7 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
.get_ethtool_stats = sja1105_get_ethtool_stats,
.get_sset_count = sja1105_get_sset_count,
.get_ts_info = sja1105_get_ts_info,
+ .port_enable = sja1105_port_enable,
.port_fdb_dump = sja1105_fdb_dump,
.port_fdb_add = sja1105_fdb_add,
.port_fdb_del = sja1105_fdb_del,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 1/2] net: dsa: Advertise the VLAN offload netdev ability only if switch supports it
From: Vladimir Oltean @ 2019-08-25 19:46 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190825194630.12404-1-olteanv@gmail.com>
When adding a VLAN sub-interface on a DSA slave port, the 8021q core
checks NETIF_F_HW_VLAN_CTAG_FILTER and, if the netdev is capable of
filtering, calls .ndo_vlan_rx_add_vid or .ndo_vlan_rx_kill_vid to
configure the VLAN offloading.
DSA sets this up counter-intuitively: it always advertises this netdev
feature, but the underlying driver may not actually support VLAN table
manipulation. In that case, the DSA core is forced to ignore the error,
because not being able to offload the VLAN is still fine - and should
result in the creation of a non-accelerated VLAN sub-interface.
Change this so that the netdev feature is only advertised for switch
drivers that support VLAN manipulation, instead of checking for
-EOPNOTSUPP at runtime.
Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/slave.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d84225125099..9a88035517a6 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1131,11 +1131,11 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
}
ret = dsa_port_vid_add(dp, vid, 0);
- if (ret && ret != -EOPNOTSUPP)
+ if (ret)
return ret;
ret = dsa_port_vid_add(dp->cpu_dp, vid, 0);
- if (ret && ret != -EOPNOTSUPP)
+ if (ret)
return ret;
return 0;
@@ -1164,14 +1164,10 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
return -EBUSY;
}
- ret = dsa_port_vid_del(dp, vid);
- if (ret == -EOPNOTSUPP)
- ret = 0;
-
/* Do not deprogram the CPU port as it may be shared with other user
* ports which can be members of this VLAN as well.
*/
- return ret;
+ return dsa_port_vid_del(dp, vid);
}
static const struct ethtool_ops dsa_slave_ethtool_ops = {
@@ -1418,8 +1414,9 @@ int dsa_slave_create(struct dsa_port *port)
if (slave_dev == NULL)
return -ENOMEM;
- slave_dev->features = master->vlan_features | NETIF_F_HW_TC |
- NETIF_F_HW_VLAN_CTAG_FILTER;
+ slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
+ if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
+ slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
slave_dev->hw_features |= NETIF_F_HW_TC;
slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
if (!IS_ERR_OR_NULL(port->mac))
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 0/2] Simplify DSA handling of VLAN subinterface offload
From: Vladimir Oltean @ 2019-08-25 19:46 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
Depends on Vivien Didelot's patchset:
https://patchwork.ozlabs.org/project/netdev/list/?series=127197&state=*
This patchset removes a few strange-looking guards for -EOPNOTSUPP in
dsa_slave_vlan_rx_add_vid and dsa_slave_vlan_rx_kill_vid, making that
code path no longer possible.
It also disables the code path for the sja1105 driver, which does
support editing the VLAN table, but not hardware-accelerated VLAN
sub-interfaces, therefore the check in the DSA core would be wrong.
There was no better DSA callback to do this than .port_enable, i.e.
at ndo_open time.
Vladimir Oltean (2):
net: dsa: Advertise the VLAN offload netdev ability only if switch
supports it
net: dsa: sja1105: Clear VLAN filtering offload netdev feature
drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++++++++++++++
net/dsa/slave.c | 15 ++++++---------
2 files changed, 22 insertions(+), 9 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] net: dsa: tag_8021q: Restore bridge VLANs when enabling vlan_filtering
From: Vladimir Oltean @ 2019-08-25 18:52 UTC (permalink / raw)
To: Florian Fainelli, Vivien Didelot, Andrew Lunn, Ido Schimmel,
Roopa Prabhu, nikolay, David S. Miller
Cc: netdev
In-Reply-To: <20190825184454.14678-3-olteanv@gmail.com>
On Sun, 25 Aug 2019 at 21:46, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> The bridge core assumes that enabling/disabling vlan_filtering will
> translate into the simple toggling of a flag for switchdev drivers.
>
> That is clearly not the case for sja1105, which alters the VLAN table
> and the pvids in order to obtain port separation in standalone mode.
>
> There are 2 parts to the issue.
>
> First, tag_8021q changes the pvid to a unique per-port rx_vid for frame
> identification. But we need to disable tag_8021q when vlan_filtering
> kicks in, and at that point, the VLAN configured as pvid will have to be
> removed from the filtering table of the ports. With an invalid pvid, the
> ports will drop all traffic. Since the bridge will not call any vlan
> operation through switchdev after enabling vlan_filtering, we need to
> ensure we're in a functional state ourselves. Hence read the pvid that
> the bridge is aware of, and program that into our ports.
>
> Secondly, tag_8021q uses the 1024-3071 range privately in
> vlan_filtering=0 mode. Had the user installed one of these VLANs during
> a previous vlan_filtering=1 session, then upon the next tag_8021q
> cleanup for vlan_filtering to kick in again, VLANs in that range will
> get deleted unconditionally, hence breaking user expectation. So when
> deleting the VLANs, check if the bridge had knowledge about them, and if
> it did, re-apply the settings. Wrap this logic inside a
> dsa_8021q_vid_apply helper function to reduce code duplication.
>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> ---
> net/dsa/tag_8021q.c | 91 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 71 insertions(+), 20 deletions(-)
>
> diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
> index 67a1bc635a7b..81f943e365b9 100644
> --- a/net/dsa/tag_8021q.c
> +++ b/net/dsa/tag_8021q.c
> @@ -93,6 +93,68 @@ int dsa_8021q_rx_source_port(u16 vid)
> }
> EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
>
> +static int dsa_8021q_restore_pvid(struct dsa_switch *ds, int port)
> +{
> + struct bridge_vlan_info vinfo;
> + struct net_device *slave;
> + u16 pvid;
> + int err;
> +
> + if (!dsa_is_user_port(ds, port))
> + return 0;
> +
> + slave = ds->ports[port].slave;
> +
> + err = br_vlan_get_pvid(slave, &pvid);
> + if (err < 0) {
> + dev_err(ds->dev, "Couldn't determine bridge PVID\n");
> + return err;
I now realize that I shouldn't error out here, since it is not an
error to not have a pvid on the port. Will make this change in v3,
along with the other change requests that will arise upon review.
> + }
> +
> + err = br_vlan_get_info(slave, pvid, &vinfo);
> + if (err < 0) {
> + dev_err(ds->dev, "Couldn't determine PVID attributes\n");
> + return err;
> + }
> +
> + return dsa_port_vid_add(&ds->ports[port], pvid, vinfo.flags);
> +}
> +
> +/* If @enabled is true, installs @vid with @flags into the switch port's HW
> + * filter.
> + * If @enabled is false, deletes @vid (ignores @flags) from the port. Had the
> + * user explicitly configured this @vid through the bridge core, then the @vid
> + * is installed again, but this time with the flags from the bridge layer.
> + */
> +static int dsa_8021q_vid_apply(struct dsa_switch *ds, int port, u16 vid,
> + u16 flags, bool enabled)
> +{
> + struct dsa_port *dp = &ds->ports[port];
> + struct bridge_vlan_info vinfo;
> + int err;
> +
> + if (enabled)
> + return dsa_port_vid_add(dp, vid, flags);
> +
> + err = dsa_port_vid_del(dp, vid);
> + if (err < 0)
> + return err;
> +
> + /* Nothing to restore from the bridge for a non-user port */
> + if (!dsa_is_user_port(ds, port))
> + return 0;
> +
> + err = br_vlan_get_info(dp->slave, vid, &vinfo);
> + /* Couldn't determine bridge attributes for this vid,
> + * it means the bridge had not configured it.
> + */
> + if (err < 0)
> + return 0;
> +
> + /* Restore the VID from the bridge */
> + return dsa_port_vid_add(dp, vid, vinfo.flags);
> +}
> +
> /* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
> * front-panel switch port (here swp0).
> *
> @@ -148,8 +210,6 @@ EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
> int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> {
> int upstream = dsa_upstream_port(ds, port);
> - struct dsa_port *dp = &ds->ports[port];
> - struct dsa_port *upstream_dp = &ds->ports[upstream];
> u16 rx_vid = dsa_8021q_rx_vid(ds, port);
> u16 tx_vid = dsa_8021q_tx_vid(ds, port);
> int i, err;
> @@ -166,7 +226,6 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> * restrictions, so there are no concerns about leaking traffic.
> */
> for (i = 0; i < ds->num_ports; i++) {
> - struct dsa_port *other_dp = &ds->ports[i];
> u16 flags;
>
> if (i == upstream)
> @@ -179,10 +238,7 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> /* The RX VID is a regular VLAN on all others */
> flags = BRIDGE_VLAN_INFO_UNTAGGED;
>
> - if (enabled)
> - err = dsa_port_vid_add(other_dp, rx_vid, flags);
> - else
> - err = dsa_port_vid_del(other_dp, rx_vid);
> + err = dsa_8021q_vid_apply(ds, i, rx_vid, flags, enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply RX VID %d to port %d: %d\n",
> rx_vid, port, err);
> @@ -193,10 +249,7 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> /* CPU port needs to see this port's RX VID
> * as tagged egress.
> */
> - if (enabled)
> - err = dsa_port_vid_add(upstream_dp, rx_vid, 0);
> - else
> - err = dsa_port_vid_del(upstream_dp, rx_vid);
> + err = dsa_8021q_vid_apply(ds, upstream, rx_vid, 0, enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply RX VID %d to port %d: %d\n",
> rx_vid, port, err);
> @@ -204,26 +257,24 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> }
>
> /* Finally apply the TX VID on this port and on the CPU port */
> - if (enabled)
> - err = dsa_port_vid_add(dp, tx_vid, BRIDGE_VLAN_INFO_UNTAGGED);
> - else
> - err = dsa_port_vid_del(dp, tx_vid);
> + err = dsa_8021q_vid_apply(ds, port, tx_vid, BRIDGE_VLAN_INFO_UNTAGGED,
> + enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply TX VID %d on port %d: %d\n",
> tx_vid, port, err);
> return err;
> }
> - if (enabled)
> - err = dsa_port_vid_add(upstream_dp, tx_vid, 0);
> - else
> - err = dsa_port_vid_del(upstream_dp, tx_vid);
> + err = dsa_8021q_vid_apply(ds, upstream, tx_vid, 0, enabled);
> if (err) {
> dev_err(ds->dev, "Failed to apply TX VID %d on port %d: %d\n",
> tx_vid, upstream, err);
> return err;
> }
>
> - return 0;
> + if (!enabled)
> + err = dsa_8021q_restore_pvid(ds, port);
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(dsa_port_setup_8021q_tagging);
>
> --
> 2.17.1
>
Thanks!
-Vladimir
^ permalink raw reply
* [PATCH v2 net-next 2/2] net: dsa: tag_8021q: Restore bridge VLANs when enabling vlan_filtering
From: Vladimir Oltean @ 2019-08-25 18:44 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, idosch, roopa, nikolay, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190825184454.14678-1-olteanv@gmail.com>
The bridge core assumes that enabling/disabling vlan_filtering will
translate into the simple toggling of a flag for switchdev drivers.
That is clearly not the case for sja1105, which alters the VLAN table
and the pvids in order to obtain port separation in standalone mode.
There are 2 parts to the issue.
First, tag_8021q changes the pvid to a unique per-port rx_vid for frame
identification. But we need to disable tag_8021q when vlan_filtering
kicks in, and at that point, the VLAN configured as pvid will have to be
removed from the filtering table of the ports. With an invalid pvid, the
ports will drop all traffic. Since the bridge will not call any vlan
operation through switchdev after enabling vlan_filtering, we need to
ensure we're in a functional state ourselves. Hence read the pvid that
the bridge is aware of, and program that into our ports.
Secondly, tag_8021q uses the 1024-3071 range privately in
vlan_filtering=0 mode. Had the user installed one of these VLANs during
a previous vlan_filtering=1 session, then upon the next tag_8021q
cleanup for vlan_filtering to kick in again, VLANs in that range will
get deleted unconditionally, hence breaking user expectation. So when
deleting the VLANs, check if the bridge had knowledge about them, and if
it did, re-apply the settings. Wrap this logic inside a
dsa_8021q_vid_apply helper function to reduce code duplication.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/tag_8021q.c | 91 +++++++++++++++++++++++++++++++++++----------
1 file changed, 71 insertions(+), 20 deletions(-)
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 67a1bc635a7b..81f943e365b9 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -93,6 +93,68 @@ int dsa_8021q_rx_source_port(u16 vid)
}
EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
+static int dsa_8021q_restore_pvid(struct dsa_switch *ds, int port)
+{
+ struct bridge_vlan_info vinfo;
+ struct net_device *slave;
+ u16 pvid;
+ int err;
+
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
+ slave = ds->ports[port].slave;
+
+ err = br_vlan_get_pvid(slave, &pvid);
+ if (err < 0) {
+ dev_err(ds->dev, "Couldn't determine bridge PVID\n");
+ return err;
+ }
+
+ err = br_vlan_get_info(slave, pvid, &vinfo);
+ if (err < 0) {
+ dev_err(ds->dev, "Couldn't determine PVID attributes\n");
+ return err;
+ }
+
+ return dsa_port_vid_add(&ds->ports[port], pvid, vinfo.flags);
+}
+
+/* If @enabled is true, installs @vid with @flags into the switch port's HW
+ * filter.
+ * If @enabled is false, deletes @vid (ignores @flags) from the port. Had the
+ * user explicitly configured this @vid through the bridge core, then the @vid
+ * is installed again, but this time with the flags from the bridge layer.
+ */
+static int dsa_8021q_vid_apply(struct dsa_switch *ds, int port, u16 vid,
+ u16 flags, bool enabled)
+{
+ struct dsa_port *dp = &ds->ports[port];
+ struct bridge_vlan_info vinfo;
+ int err;
+
+ if (enabled)
+ return dsa_port_vid_add(dp, vid, flags);
+
+ err = dsa_port_vid_del(dp, vid);
+ if (err < 0)
+ return err;
+
+ /* Nothing to restore from the bridge for a non-user port */
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
+ err = br_vlan_get_info(dp->slave, vid, &vinfo);
+ /* Couldn't determine bridge attributes for this vid,
+ * it means the bridge had not configured it.
+ */
+ if (err < 0)
+ return 0;
+
+ /* Restore the VID from the bridge */
+ return dsa_port_vid_add(dp, vid, vinfo.flags);
+}
+
/* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
* front-panel switch port (here swp0).
*
@@ -148,8 +210,6 @@ EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
{
int upstream = dsa_upstream_port(ds, port);
- struct dsa_port *dp = &ds->ports[port];
- struct dsa_port *upstream_dp = &ds->ports[upstream];
u16 rx_vid = dsa_8021q_rx_vid(ds, port);
u16 tx_vid = dsa_8021q_tx_vid(ds, port);
int i, err;
@@ -166,7 +226,6 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
* restrictions, so there are no concerns about leaking traffic.
*/
for (i = 0; i < ds->num_ports; i++) {
- struct dsa_port *other_dp = &ds->ports[i];
u16 flags;
if (i == upstream)
@@ -179,10 +238,7 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
/* The RX VID is a regular VLAN on all others */
flags = BRIDGE_VLAN_INFO_UNTAGGED;
- if (enabled)
- err = dsa_port_vid_add(other_dp, rx_vid, flags);
- else
- err = dsa_port_vid_del(other_dp, rx_vid);
+ err = dsa_8021q_vid_apply(ds, i, rx_vid, flags, enabled);
if (err) {
dev_err(ds->dev, "Failed to apply RX VID %d to port %d: %d\n",
rx_vid, port, err);
@@ -193,10 +249,7 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
/* CPU port needs to see this port's RX VID
* as tagged egress.
*/
- if (enabled)
- err = dsa_port_vid_add(upstream_dp, rx_vid, 0);
- else
- err = dsa_port_vid_del(upstream_dp, rx_vid);
+ err = dsa_8021q_vid_apply(ds, upstream, rx_vid, 0, enabled);
if (err) {
dev_err(ds->dev, "Failed to apply RX VID %d to port %d: %d\n",
rx_vid, port, err);
@@ -204,26 +257,24 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
}
/* Finally apply the TX VID on this port and on the CPU port */
- if (enabled)
- err = dsa_port_vid_add(dp, tx_vid, BRIDGE_VLAN_INFO_UNTAGGED);
- else
- err = dsa_port_vid_del(dp, tx_vid);
+ err = dsa_8021q_vid_apply(ds, port, tx_vid, BRIDGE_VLAN_INFO_UNTAGGED,
+ enabled);
if (err) {
dev_err(ds->dev, "Failed to apply TX VID %d on port %d: %d\n",
tx_vid, port, err);
return err;
}
- if (enabled)
- err = dsa_port_vid_add(upstream_dp, tx_vid, 0);
- else
- err = dsa_port_vid_del(upstream_dp, tx_vid);
+ err = dsa_8021q_vid_apply(ds, upstream, tx_vid, 0, enabled);
if (err) {
dev_err(ds->dev, "Failed to apply TX VID %d on port %d: %d\n",
tx_vid, upstream, err);
return err;
}
- return 0;
+ if (!enabled)
+ err = dsa_8021q_restore_pvid(ds, port);
+
+ return err;
}
EXPORT_SYMBOL_GPL(dsa_port_setup_8021q_tagging);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 1/2] net: bridge: Populate the pvid flag in br_vlan_get_info
From: Vladimir Oltean @ 2019-08-25 18:44 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, idosch, roopa, nikolay, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190825184454.14678-1-olteanv@gmail.com>
Currently this simplified code snippet fails:
br_vlan_get_pvid(netdev, &pvid);
br_vlan_get_info(netdev, pvid, &vinfo);
ASSERT(!(vinfo.flags & BRIDGE_VLAN_INFO_PVID));
It is intuitive that the pvid of a netdevice should have the
BRIDGE_VLAN_INFO_PVID flag set.
However I can't seem to pinpoint a commit where this behavior was
introduced. It seems like it's been like that since forever.
At a first glance it would make more sense to just handle the
BRIDGE_VLAN_INFO_PVID flag in __vlan_add_flags. However, as Nikolay
explains:
There are a few reasons why we don't do it, most importantly because
we need to have only one visible pvid at any single time, even if it's
stale - it must be just one. Right now that rule will not be violated
by this change, but people will try using this flag and could see two
pvids simultaneously. You can see that the pvid code is even using
memory barriers to propagate the new value faster and everywhere the
pvid is read only once. That is the reason the flag is set
dynamically when dumping entries, too. A second (weaker) argument
against would be given the above we don't want another way to do the
same thing, specifically if it can provide us with two pvids (e.g. if
walking the vlan list) or if it can provide us with a pvid different
from the one set in the vg. [Obviously, I'm talking about RCU
pvid/vlan use cases similar to the dumps. The locked cases are fine.
I would like to avoid explaining why this shouldn't be relied upon
without locking]
So instead of introducing the above change and making sure of the pvid
uniqueness under RCU, simply dynamically populate the pvid flag in
br_vlan_get_info().
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_vlan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f5b2aeebbfe9..bb98984cd27d 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1281,6 +1281,8 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
p_vinfo->vid = vid;
p_vinfo->flags = v->flags;
+ if (vid == br_get_pvid(vg))
+ p_vinfo->flags |= BRIDGE_VLAN_INFO_PVID;
return 0;
}
EXPORT_SYMBOL_GPL(br_vlan_get_info);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 0/2] Dynamic toggling of vlan_filtering for SJA1105 DSA
From: Vladimir Oltean @ 2019-08-25 18:44 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, idosch, roopa, nikolay, davem
Cc: netdev, Vladimir Oltean
This patchset addresses a limitation in dsa_8021q where this sequence of
commands was causing the switch to stop forwarding traffic:
ip link add name br0 type bridge vlan_filtering 0
ip link set dev swp2 master br0
echo 1 > /sys/class/net/br0/bridge/vlan_filtering
echo 0 > /sys/class/net/br0/bridge/vlan_filtering
The issue has to do with the VLAN table manipulations that dsa_8021q
does without notifying the bridge layer. The solution is to always
restore the VLANs that the bridge knows about, when disabling tagging.
Depends on Vivien Didelot's patchset:
https://patchwork.ozlabs.org/project/netdev/list/?series=127197&state=*
Also see this discussion thread:
https://www.spinics.net/lists/netdev/msg581042.html
Vladimir Oltean (2):
net: bridge: Populate the pvid flag in br_vlan_get_info
net: dsa: tag_8021q: Restore bridge VLANs when enabling vlan_filtering
net/bridge/br_vlan.c | 2 +
net/dsa/tag_8021q.c | 91 ++++++++++++++++++++++++++++++++++----------
2 files changed, 73 insertions(+), 20 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net] net: dsa: tag_8021q: Future-proof the reserved fields in the custom VID
From: Vladimir Oltean @ 2019-08-25 18:32 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
After witnessing the discussion in https://lkml.org/lkml/2019/8/14/151
w.r.t. ioctl extensibility, it became clear that such an issue might
prevent that the 3 RSV bits inside the DSA 802.1Q tag might also suffer
the same fate and be useless for further extension.
So clearly specify that the reserved bits should currently be
transmitted as zero and ignored on receive. The DSA tagger already does
this (and has always did), and is the only known user so far (no
Wireshark dissection plugin, etc). So there should be no incompatibility
to speak of.
Fixes: 0471dd429cea ("net: dsa: tag_8021q: Create a stable binary format")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/tag_8021q.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 6ebbd799c4eb..67a1bc635a7b 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -28,6 +28,7 @@
*
* RSV - VID[9]:
* To be used for further expansion of SWITCH_ID or for other purposes.
+ * Must be transmitted as zero and ignored on receive.
*
* SWITCH_ID - VID[8:6]:
* Index of switch within DSA tree. Must be between 0 and
@@ -35,6 +36,7 @@
*
* RSV - VID[5:4]:
* To be used for further expansion of PORT or for other purposes.
+ * Must be transmitted as zero and ignored on receive.
*
* PORT - VID[3:0]:
* Index of switch port. Must be between 0 and DSA_MAX_PORTS - 1.
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 29/38] cls_flower: Convert handle_idr to XArray
From: Cong Wang @ 2019-08-25 18:32 UTC (permalink / raw)
To: Vlad Buslov; +Cc: Matthew Wilcox, netdev@vger.kernel.org
In-Reply-To: <vbftvaa4bny.fsf@mellanox.com>
On Wed, Aug 21, 2019 at 11:27 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> At first I was confused why you bring up rtnl lock in commit message
> (flower classifier has 'unlocked' flag set and can't rely on it anymore)
> but looking at the code I see that we lost rcu read lock here in commit
> d39d714969cd ("idr: introduce idr_for_each_entry_continue_ul()") and you
> are correctly bringing it back. Adding Cong to advise if it is okay to
> wait for this patch to be accepted or we need to proceed with fixing the
> missing RCU lock as a standalone patch.
Hmm? Isn't ->walk() still called with RTNL lock? tcf_chain_dump()
calls __tcf_get_next_proto() which asserts RTNL.
So why does it still need RCU read lock when having RTNL?
^ 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