* Re: linux-next: Fixes tag needs some work in the net-next tree
From: Jiri Pirko @ 2019-02-20 20:26 UTC (permalink / raw)
To: David Miller; +Cc: vkoul, sfr, netdev, linux-next, linux-kernel, andrew
In-Reply-To: <20190220.122522.1237657062289636264.davem@davemloft.net>
Wed, Feb 20, 2019 at 09:25:22PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Wed, 20 Feb 2019 20:58:11 +0100
>
>> Wed, Feb 20, 2019 at 08:02:01PM CET, davem@davemloft.net wrote:
>>>From: Jiri Pirko <jiri@resnulli.us>
>>>Date: Wed, 20 Feb 2019 09:36:11 +0100
>>>
>>>> Would be good to have some robot checking "Fixes" sanity...
>>>
>>>I want to add a script to my trees that locally do it for me but the
>>
>> When it is in tree, it's already late. The robot has to check
>> emails/patchwork.
>
>It's for me, on my computer, to abort things when I try to commit
>with a bad fixes tag or missing signoff.
Okay, but still, robot checking emails/patchwork would safe you some
overhead.
^ permalink raw reply
* Re: linux-next: Fixes tag needs some work in the net-next tree
From: Stefano Brivio @ 2019-02-20 20:37 UTC (permalink / raw)
To: David Miller; +Cc: jiri, vkoul, sfr, netdev, linux-next, linux-kernel, andrew
In-Reply-To: <20190220.122548.2257661716734858013.davem@davemloft.net>
On Wed, 20 Feb 2019 12:25:48 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> From: Stefano Brivio <sbrivio@redhat.com>
> Date: Wed, 20 Feb 2019 20:59:10 +0100
>
> > On Wed, 20 Feb 2019 11:02:01 -0800 (PST)
> > David Miller <davem@davemloft.net> wrote:
> >
> >> From: Jiri Pirko <jiri@resnulli.us>
> >> Date: Wed, 20 Feb 2019 09:36:11 +0100
> >>
> >> > Would be good to have some robot checking "Fixes" sanity...
> >>
> >> I want to add a script to my trees that locally do it for me but the
> >> backlog for patch review for me is so huge that I never get to "fun"
> >> tasks like that....
> >
> > If it helps, this is what I use after being bitten once:
>
> Awesome, thanks! You put this in as a pre-commit script?
Nope, I use it before sending patches out with git send-email.
But you can use this as .git/hooks/commit-msg, just skip taking the
tree as second argument, something like this (just quickly tested):
#!/bin/sh
grep "^Fixes: " "${1}" | while read -r f; do
sha="$(echo "${f}" | cut -d' ' -f2)"
if [ -z "${sha}" ] || [ "${f}" != "$(git show -s --abbrev=12 --pretty=format:"Fixes: %h (\"%s\")" "${sha}" 2>/dev/null)" ]; then
echo "Bad tag: ${f}" && exit 1
fi
done
--
Stefano
^ permalink raw reply
* Re: [PATCH 6/6] net: ethernet: ti: cpsw: deprecate cpsw-phy-sel driver
From: David Miller @ 2019-02-20 20:41 UTC (permalink / raw)
To: tony
Cc: grygorii.strashko, kishon, robh+dt, netdev, nsekhar, linux-kernel,
linux-omap, devicetree, linux-arm-kernel
In-Reply-To: <20190220203326.GT15711@atomide.com>
From: Tony Lindgren <tony@atomide.com>
Date: Wed, 20 Feb 2019 12:33:26 -0800
> * David Miller <davem@davemloft.net> [190220 19:23]:
>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>> Date: Wed, 20 Feb 2019 17:25:19 +0200
>>
>> > Deprecate cpsw-phy-sel driver as it's been replaced with new
>> > TI phy-gmii-sel PHY driver.
>> >
>> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> Acked-by: David S. Miller <davem@davemloft.net>
>
> Thanks for the ack, but actually I'd prefer you to pick
> this patch at some future date. I suggest Grygorii just
> repost this one after v5.1-rc1.
It's so much easier if you group this with those DT changes, they
logically belong together as well and it helps someone reading
the changes in the tree also if they are side by side.
Why don't you want to integrate this with them?
^ permalink raw reply
* Re: linux-next: Fixes tag needs some work in the net-next tree
From: David Miller @ 2019-02-20 20:42 UTC (permalink / raw)
To: sbrivio; +Cc: jiri, vkoul, sfr, netdev, linux-next, linux-kernel, andrew
In-Reply-To: <20190220213729.49deb54f@redhat.com>
From: Stefano Brivio <sbrivio@redhat.com>
Date: Wed, 20 Feb 2019 21:37:29 +0100
> On Wed, 20 Feb 2019 12:25:48 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Stefano Brivio <sbrivio@redhat.com>
>> Date: Wed, 20 Feb 2019 20:59:10 +0100
>>
>> > On Wed, 20 Feb 2019 11:02:01 -0800 (PST)
>> > David Miller <davem@davemloft.net> wrote:
>> >
>> >> From: Jiri Pirko <jiri@resnulli.us>
>> >> Date: Wed, 20 Feb 2019 09:36:11 +0100
>> >>
>> >> > Would be good to have some robot checking "Fixes" sanity...
>> >>
>> >> I want to add a script to my trees that locally do it for me but the
>> >> backlog for patch review for me is so huge that I never get to "fun"
>> >> tasks like that....
>> >
>> > If it helps, this is what I use after being bitten once:
>>
>> Awesome, thanks! You put this in as a pre-commit script?
>
> Nope, I use it before sending patches out with git send-email.
>
> But you can use this as .git/hooks/commit-msg, just skip taking the
> tree as second argument, something like this (just quickly tested):
Awesome, thank you!
^ permalink raw reply
* [PATCH net-next v4 0/3] net: dsa: mv88e6xxx: fix IPv6
From: Russell King - ARM Linux admin @ 2019-02-20 20:55 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot
Cc: David S. Miller, Heiner Kallweit, netdev
We have had some emails in private over this issue, this is my current
patch set rebased on top of net-next which provides working IPv6 (and
probably other protocols as well) over mv88e6xxx DSA switches.
The problem comes down to mv88e6xxx defaulting to not flood unknown
unicast and multicast datagrams, as they would be by dumb switches,
and as the Linux bridge code does by default.
There is also the issue of IPv6 over a vlan that is transparent to the
bridge; the multicast querier will not reach inside the vlan, and so
the switch can not learn about multicast routing within the vlan.
These flood settings can be disabled via the Linux bridge code if it's
desired to make the switch behave more like a managed switch, eg, by
enabling the multicast querier. However, the multicast querier
defaults to being disabled which effectively means that by default,
mv88e6xxx switches block all multicast traffic. This is at odds with
the Linux bridge documentation, and the defaults that the Linux bridge
code adopts.
So, this patch set adds DSA support for Linux bridge flags, adds
mv88e6xxx support for the unicast and multicast flooding flags, and
lastly enables flooding of these frames by default to match the
Linux bridge defaults.
drivers/net/dsa/mv88e6xxx/chip.c | 17 +++++++++++++++++
include/net/dsa.h | 2 ++
net/dsa/dsa_priv.h | 2 ++
net/dsa/port.c | 33 ++++++++++++++++++++++++++++++---
net/dsa/slave.c | 6 ++++++
5 files changed, 57 insertions(+), 3 deletions(-)
v2: fix a couple of compile errors in patch 2 and patch 3 (oops).
v3: change interface between core DSA and drivers
v4: fix comments from v3
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH net-next v4 1/3] net: dsa: add support for bridge flags
From: Russell King @ 2019-02-20 20:55 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot
Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190220205525.nji63ntsthxbus4l@shell.armlinux.org.uk>
The Linux bridge implementation allows various properties of the bridge
to be controlled, such as flooding unknown unicast and multicast frames.
This patch adds the necessary DSA infrastructure to allow the Linux
bridge support to control these properties for DSA switches.
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
include/net/dsa.h | 2 ++
net/dsa/dsa_priv.h | 2 ++
net/dsa/port.c | 17 +++++++++++++++++
net/dsa/slave.c | 6 ++++++
4 files changed, 27 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7f2a668ef2cc..2c2c10812814 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -400,6 +400,8 @@ struct dsa_switch_ops {
void (*port_stp_state_set)(struct dsa_switch *ds, int port,
u8 state);
void (*port_fast_age)(struct dsa_switch *ds, int port);
+ int (*port_egress_floods)(struct dsa_switch *ds, int port,
+ bool unicast, bool multicast);
/*
* VLAN support
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 1f4972dab9f2..f4f99ec29f5d 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -160,6 +160,8 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
struct switchdev_trans *trans);
int dsa_port_mdb_del(const struct dsa_port *dp,
const struct switchdev_obj_port_mdb *mdb);
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
+ struct switchdev_trans *trans);
int dsa_port_vlan_add(struct dsa_port *dp,
const struct switchdev_obj_port_vlan *vlan,
struct switchdev_trans *trans);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2d7e01b23572..6df29bddf37e 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -177,6 +177,23 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
}
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
+ struct switchdev_trans *trans)
+{
+ struct dsa_switch *ds = dp->ds;
+ int port = dp->index;
+ int err = 0;
+
+ if (switchdev_trans_ph_prepare(trans))
+ return 0;
+
+ if (ds->ops->port_egress_floods)
+ err = ds->ops->port_egress_floods(ds, port, flags & BR_FLOOD,
+ flags & BR_MCAST_FLOOD);
+
+ return err;
+}
+
int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
u16 vid)
{
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e5e7c04821b..f99161c3b1ea 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -295,6 +295,9 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
ret = dsa_port_ageing_time(dp, attr->u.ageing_time, trans);
break;
+ case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
+ ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, trans);
+ break;
default:
ret = -EOPNOTSUPP;
break;
@@ -384,6 +387,9 @@ static int dsa_slave_port_attr_get(struct net_device *dev,
switch (attr->id) {
case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
attr->u.brport_flags_support = 0;
+ if (ds->ops->port_egress_floods)
+ attr->u.brport_flags_support |= BR_FLOOD |
+ BR_MCAST_FLOOD;
break;
default:
return -EOPNOTSUPP;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Russell King @ 2019-02-20 20:55 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot
Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190220205525.nji63ntsthxbus4l@shell.armlinux.org.uk>
Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
the multicast and unicast flood properties to be controlled. These
can be controlled on a per-port basis via commands such as:
bridge link set dev lan1 flood on|off
bridge link set dev lan1 mcast_flood on|off
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/mv88e6xxx/chip.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 32e7af5caa69..cc7ce06b6d58 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4692,6 +4692,22 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
return err;
}
+static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
+ bool unicast, bool multicast)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int err = -EOPNOTSUPP;
+
+ mutex_lock(&chip->reg_lock);
+ if (chip->info->ops->port_set_egress_floods)
+ err = chip->info->ops->port_set_egress_floods(chip, port,
+ unicast,
+ multicast);
+ mutex_unlock(&chip->reg_lock);
+
+ return err;
+}
+
static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
#if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
.probe = mv88e6xxx_drv_probe,
@@ -4719,6 +4735,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.set_ageing_time = mv88e6xxx_set_ageing_time,
.port_bridge_join = mv88e6xxx_port_bridge_join,
.port_bridge_leave = mv88e6xxx_port_bridge_leave,
+ .port_egress_floods = mv88e6xxx_port_egress_floods,
.port_stp_state_set = mv88e6xxx_port_stp_state_set,
.port_fast_age = mv88e6xxx_port_fast_age,
.port_vlan_filtering = mv88e6xxx_port_vlan_filtering,
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 3/3] net: dsa: enable flooding for bridge ports
From: Russell King @ 2019-02-20 20:56 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot
Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190220205525.nji63ntsthxbus4l@shell.armlinux.org.uk>
Switches work by learning the MAC address for each attached station by
monitoring traffic from each station. When a station sends a packet,
the switch records which port the MAC address is connected to.
With IPv4 networking, before communication commences with a neighbour,
an ARP packet is broadcasted to all stations asking for the MAC address
corresponding with the IPv4. The desired station responds with an ARP
reply, and the ARP reply causes the switch to learn which port the
station is connected to.
With IPv6 networking, the situation is rather different. Rather than
broadcasting ARP packets, a "neighbour solicitation" is multicasted
rather than broadcasted. This multicast needs to reach the intended
station in order for the neighbour to be discovered.
Once a neighbour has been discovered, and entered into the sending
stations neighbour cache, communication can restart at a point later
without sending a new neighbour solicitation, even if the entry in
the neighbour cache is marked as stale. This can be after the MAC
address has expired from the forwarding cache of the DSA switch -
when that occurs, there is a long pause in communication.
Our DSA implementation for mv88e6xxx switches disables flooding of
multicast and unicast frames for bridged ports. As per the above
description, this is fine for IPv4 networking, since the broadcasted
ARP queries will be sent to and received by all stations on the same
network. However, this breaks IPv6 very badly - blocking neighbour
solicitations and later causing connections to stall.
The defaults that the Linux bridge code expect from bridges are for
unknown unicast and unknown multicast frames to be flooded to all ports
on the bridge, which is at odds to the defaults adopted by our DSA
implementation for mv88e6xxx switches.
This commit enables by default flooding of both unknown unicast and
unknown multicast frames whenever a port is added to a bridge, and
disables the flooding when a port leaves the bridge. This means that
mv88e6xxx DSA switches now behave as per the bridge(8) man page, and
IPv6 works flawlessly through such a switch.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
net/dsa/port.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 6df29bddf37e..7bc2a5ad95c6 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -105,16 +105,23 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
};
int err;
- /* Here the port is already bridged. Reflect the current configuration
- * so that drivers can program their chips accordingly.
+ /* Set the flooding mode before joining the port in the switch */
+ err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD, NULL);
+ if (err)
+ return err;
+
+ /* Here the interface is already bridged. Reflect the current
+ * configuration so that drivers can program their chips accordingly.
*/
dp->bridge_dev = br;
err = dsa_port_notify(dp, DSA_NOTIFIER_BRIDGE_JOIN, &info);
/* The bridging is rolled back on error */
- if (err)
+ if (err) {
+ dsa_port_bridge_flags(dp, 0, NULL);
dp->bridge_dev = NULL;
+ }
return err;
}
@@ -137,6 +144,9 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
if (err)
pr_err("DSA: failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n");
+ /* Port is leaving the bridge, disable flooding */
+ dsa_port_bridge_flags(dp, 0, NULL);
+
/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
* so allow it to be in BR_STATE_FORWARDING to be kept functional
*/
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 6/6] net: ethernet: ti: cpsw: deprecate cpsw-phy-sel driver
From: Tony Lindgren @ 2019-02-20 21:01 UTC (permalink / raw)
To: David Miller
Cc: grygorii.strashko, kishon, robh+dt, netdev, nsekhar, linux-kernel,
linux-omap, devicetree, linux-arm-kernel
In-Reply-To: <20190220.124155.2226787912266109280.davem@davemloft.net>
* David Miller <davem@davemloft.net> [190220 20:42]:
> From: Tony Lindgren <tony@atomide.com>
> Date: Wed, 20 Feb 2019 12:33:26 -0800
>
> > * David Miller <davem@davemloft.net> [190220 19:23]:
> >> From: Grygorii Strashko <grygorii.strashko@ti.com>
> >> Date: Wed, 20 Feb 2019 17:25:19 +0200
> >>
> >> > Deprecate cpsw-phy-sel driver as it's been replaced with new
> >> > TI phy-gmii-sel PHY driver.
> >> >
> >> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> >>
> >> Acked-by: David S. Miller <davem@davemloft.net>
> >
> > Thanks for the ack, but actually I'd prefer you to pick
> > this patch at some future date. I suggest Grygorii just
> > repost this one after v5.1-rc1.
>
> It's so much easier if you group this with those DT changes, they
> logically belong together as well and it helps someone reading
> the changes in the tree also if they are side by side.
I agree that a group of patches should go together in
most cases.
> Why don't you want to integrate this with them?
Because the arm-soc tree wants dts changes separately in
general. The dts changes are considered firmware describing
hardware. And it makes it possible to split the arm-soc pile
of patches into multiple pull requests. In theory there
should be no dependency between dts changes and driver
changes, but in reality that's not always the case :)
What I can do is set up a separate branch with just this
patch on top of the dts changes that the arm-soc guys can
then merge towards the end of the merge cycle. If that
works for you, let me know and I'll do it.
Regards,
Tony
^ permalink raw reply
* Re: bug report: iwlwifi: mvm: support mac80211 TXQs model
From: Johannes Berg @ 2019-02-20 21:12 UTC (permalink / raw)
To: Colin Ian King, Sara Sharon, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, linux-wireless@vger.kernel.org, netdev
Cc: Kalle Valo, David S. Miller, linux-kernel@vger.kernel.org
In-Reply-To: <427786ae-159a-a7d1-2334-46656d3f2b51@canonical.com>
On Wed, 2019-02-20 at 14:40 +0000, Colin Ian King wrote:
>
> ..when the used_hw_queues initialization was removed:
>
> @@ -360,8 +300,6 @@ int iwl_mvm_mac_ctxt_init(struct iwl_mvm *mvm,
> struct ieee80211_vif *vif)
> mvm->hw, IEEE80211_IFACE_ITER_RESUME_ALL,
> iwl_mvm_mac_iface_iterator, &data);
>
> - used_hw_queues = iwl_mvm_get_used_hw_queues(mvm, vif);
> -
> /*
> * In the case we're getting here during resume, it's similar to
> * firmware restart, and with RESUME_ALL the iterator will find
>
>
> I'm not 100% sure if the right thing to do is just to now initialize
> used_hw_queues to zero; it's not entirely clear what the initial value
> should be now.
My gut feeling says that we should just remove all of the code - we no
longer use the vif->hw_queue stuff. Which also explains why we didn't
notice any problems from this :)
Something like
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
index aa308f7e7989..f90383943c62 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
@@ -263,8 +263,7 @@ int iwl_mvm_mac_ctxt_init(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
.found_vif = false,
};
u32 ac;
- int ret, i, queue_limit;
- unsigned long used_hw_queues;
+ int ret, i;
lockdep_assert_held(&mvm->mutex);
@@ -341,38 +340,9 @@ int iwl_mvm_mac_ctxt_init(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
INIT_LIST_HEAD(&mvmvif->time_event_data.list);
mvmvif->time_event_data.id = TE_MAX;
- /* No need to allocate data queues to P2P Device MAC and NAN.*/
if (vif->type == NL80211_IFTYPE_P2P_DEVICE ||
- vif->type == NL80211_IFTYPE_NAN) {
- for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
- vif->hw_queue[ac] = IEEE80211_INVAL_HW_QUEUE;
-
+ vif->type == NL80211_IFTYPE_NAN)
return 0;
- }
-
- /*
- * queues in mac80211 almost entirely independent of
- * the ones here - no real limit
- */
- queue_limit = IEEE80211_MAX_QUEUES;
-
- /*
- * Find available queues, and allocate them to the ACs. When in
- * DQA-mode they aren't really used, and this is done only so the
- * mac80211 ieee80211_check_queues() function won't fail
- */
- for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
- u8 queue = find_first_zero_bit(&used_hw_queues, queue_limit);
-
- if (queue >= queue_limit) {
- IWL_ERR(mvm, "Failed to allocate queue\n");
- ret = -EIO;
- goto exit_fail;
- }
-
- __set_bit(queue, &used_hw_queues);
- vif->hw_queue[ac] = queue;
- }
/* Allocate the CAB queue for softAP and GO interfaces */
if (vif->type == NL80211_IFTYPE_AP ||
I'll think about it a bit more and test it out, thanks for the report!
johannes
^ permalink raw reply related
* [PATCH net] net: socket: add check for negative optlen in compat setsockopt
From: Jann Horn @ 2019-02-20 21:34 UTC (permalink / raw)
To: David S. Miller, jannh; +Cc: netdev, linux-kernel
__sys_setsockopt() already checks for `optlen < 0`. Add an equivalent check
to the compat path for robustness. This has to be `> INT_MAX` instead of
`< 0` because the signedness of `optlen` is different here.
Signed-off-by: Jann Horn <jannh@google.com>
---
net/compat.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/compat.c b/net/compat.c
index 959d1c51826d..3d348198004f 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -388,8 +388,12 @@ static int __compat_sys_setsockopt(int fd, int level, int optname,
char __user *optval, unsigned int optlen)
{
int err;
- struct socket *sock = sockfd_lookup(fd, &err);
+ struct socket *sock;
+
+ if (optlen > INT_MAX)
+ return -EINVAL;
+ sock = sockfd_lookup(fd, &err);
if (sock) {
err = security_socket_setsockopt(sock, level, optname);
if (err) {
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* Re: [PATCH net-next v4 1/3] net: dsa: add support for bridge flags
From: Florian Fainelli @ 2019-02-20 21:46 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Vivien Didelot
Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <E1gwYuI-0001aX-2u@rmk-PC.armlinux.org.uk>
On 2/20/19 12:55 PM, Russell King wrote:
> The Linux bridge implementation allows various properties of the bridge
> to be controlled, such as flooding unknown unicast and multicast frames.
> This patch adds the necessary DSA infrastructure to allow the Linux
> bridge support to control these properties for DSA switches.
>
> Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v4 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Florian Fainelli @ 2019-02-20 21:51 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Vivien Didelot
Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <E1gwYuN-0001as-7s@rmk-PC.armlinux.org.uk>
On 2/20/19 12:55 PM, Russell King wrote:
> Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
> the multicast and unicast flood properties to be controlled. These
> can be controlled on a per-port basis via commands such as:
>
> bridge link set dev lan1 flood on|off
> bridge link set dev lan1 mcast_flood on|off
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] net: dsa: enable flooding for bridge ports
From: Florian Fainelli @ 2019-02-20 21:51 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Vivien Didelot
Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <E1gwYuS-0001b7-L7@rmk-PC.armlinux.org.uk>
On 2/20/19 12:56 PM, Russell King wrote:
> Switches work by learning the MAC address for each attached station by
> monitoring traffic from each station. When a station sends a packet,
> the switch records which port the MAC address is connected to.
>
> With IPv4 networking, before communication commences with a neighbour,
> an ARP packet is broadcasted to all stations asking for the MAC address
> corresponding with the IPv4. The desired station responds with an ARP
> reply, and the ARP reply causes the switch to learn which port the
> station is connected to.
>
> With IPv6 networking, the situation is rather different. Rather than
> broadcasting ARP packets, a "neighbour solicitation" is multicasted
> rather than broadcasted. This multicast needs to reach the intended
> station in order for the neighbour to be discovered.
>
> Once a neighbour has been discovered, and entered into the sending
> stations neighbour cache, communication can restart at a point later
> without sending a new neighbour solicitation, even if the entry in
> the neighbour cache is marked as stale. This can be after the MAC
> address has expired from the forwarding cache of the DSA switch -
> when that occurs, there is a long pause in communication.
>
> Our DSA implementation for mv88e6xxx switches disables flooding of
> multicast and unicast frames for bridged ports. As per the above
> description, this is fine for IPv4 networking, since the broadcasted
> ARP queries will be sent to and received by all stations on the same
> network. However, this breaks IPv6 very badly - blocking neighbour
> solicitations and later causing connections to stall.
>
> The defaults that the Linux bridge code expect from bridges are for
> unknown unicast and unknown multicast frames to be flooded to all ports
> on the bridge, which is at odds to the defaults adopted by our DSA
> implementation for mv88e6xxx switches.
>
> This commit enables by default flooding of both unknown unicast and
> unknown multicast frames whenever a port is added to a bridge, and
> disables the flooding when a port leaves the bridge. This means that
> mv88e6xxx DSA switches now behave as per the bridge(8) man page, and
> IPv6 works flawlessly through such a switch.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v4 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Vivien Didelot @ 2019-02-20 22:23 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <E1gwYuN-0001as-7s@rmk-PC.armlinux.org.uk>
On Wed, 20 Feb 2019 20:55:59 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
> the multicast and unicast flood properties to be controlled. These
> can be controlled on a per-port basis via commands such as:
>
> bridge link set dev lan1 flood on|off
> bridge link set dev lan1 mcast_flood on|off
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] net: dsa: enable flooding for bridge ports
From: Vivien Didelot @ 2019-02-20 22:29 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <E1gwYuS-0001b7-L7@rmk-PC.armlinux.org.uk>
On Wed, 20 Feb 2019 20:56:04 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Switches work by learning the MAC address for each attached station by
> monitoring traffic from each station. When a station sends a packet,
> the switch records which port the MAC address is connected to.
>
> With IPv4 networking, before communication commences with a neighbour,
> an ARP packet is broadcasted to all stations asking for the MAC address
> corresponding with the IPv4. The desired station responds with an ARP
> reply, and the ARP reply causes the switch to learn which port the
> station is connected to.
>
> With IPv6 networking, the situation is rather different. Rather than
> broadcasting ARP packets, a "neighbour solicitation" is multicasted
> rather than broadcasted. This multicast needs to reach the intended
> station in order for the neighbour to be discovered.
>
> Once a neighbour has been discovered, and entered into the sending
> stations neighbour cache, communication can restart at a point later
> without sending a new neighbour solicitation, even if the entry in
> the neighbour cache is marked as stale. This can be after the MAC
> address has expired from the forwarding cache of the DSA switch -
> when that occurs, there is a long pause in communication.
>
> Our DSA implementation for mv88e6xxx switches disables flooding of
> multicast and unicast frames for bridged ports. As per the above
> description, this is fine for IPv4 networking, since the broadcasted
> ARP queries will be sent to and received by all stations on the same
> network. However, this breaks IPv6 very badly - blocking neighbour
> solicitations and later causing connections to stall.
>
> The defaults that the Linux bridge code expect from bridges are for
> unknown unicast and unknown multicast frames to be flooded to all ports
> on the bridge, which is at odds to the defaults adopted by our DSA
> implementation for mv88e6xxx switches.
>
> This commit enables by default flooding of both unknown unicast and
> unknown multicast frames whenever a port is added to a bridge, and
> disables the flooding when a port leaves the bridge. This means that
> mv88e6xxx DSA switches now behave as per the bridge(8) man page, and
> IPv6 works flawlessly through such a switch.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference
From: Cong Wang @ 2019-02-20 22:33 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <vbf8sycqfn8.fsf@mellanox.com>
On Tue, Feb 19, 2019 at 1:46 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Mon 18 Feb 2019 at 19:08, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>
> >> Flower classifier only changes root pointer during init and destroy. Cls
> >> API implements reference counting for tcf_proto, so there is no danger of
> >> concurrent access to tp when it is being destroyed, even without protection
> >> provided by rtnl lock.
> >
> > How about atomicity? Refcnt doesn't guarantee atomicity, how do
> > you make sure two concurrent modifications are atomic?
>
> In order to guarantee atomicity I lock shared flower classifier data
> structures with tp->lock in following patches.
Sure, I meant the atomicity of the _whole_ change, as you know
the TC filters are stored in hierarchical structures: a block, a chain,
a tp struct, some type-specific data structure like a hash table.
Locking tp only solves a partial of the atomicity here. Are you
going to restart the whole change from top down to the bottom?
>
> >
> >
> >>
> >> Implement new function fl_head_dereference() to dereference tp->root
> >> without checking for rtnl lock. Use it in all flower function that obtain
> >> head pointer instead of rtnl_dereference().
> >>
> >
> > So what lock protects RCU writers after this patch?
>
> I explained it in comment for fl_head_dereference(), but should have
> copied this information to changelog as well:
> Flower classifier only changes root pointer during init and destroy.
> Cls API implements reference counting for tcf_proto, so there is no
> danger of concurrent access to tp when it is being destroyed, even
> without protection provided by rtnl lock.
So you are saying an RCU pointer is okay to deference without
any lock eve without RCU read lock, right?
>
> In initial version of this change I used tp->lock to protect tp->root
> access and verified it with lockdep, but during internal review Jiri
> noted that this is not needed in current flower implementation.
Let's see what you have on top of your own branch
unlocked_flower_cong_1:
1458 static int fl_change(struct net *net, struct sk_buff *in_skb,
1459 struct tcf_proto *tp, unsigned long base,
1460 u32 handle, struct nlattr **tca,
1461 void **arg, bool ovr, bool rtnl_held,
1462 struct netlink_ext_ack *extack)
1463 {
1464 struct cls_fl_head *head = fl_head_dereference(tp);
At the point of line 1464, there is no lock taken, tp->lock is taken
after it, block->lock or chain lock is already unlocked before ->change().
So, what protects this RCU structure? According to RCU, it must be
either RCU read lock or some writer lock. I see none here. :(
What am I missing?
^ permalink raw reply
* [PATCH net-next 0/2] net: dsa: VLAN devices w/ filtering
From: Florian Fainelli @ 2019-02-20 22:35 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, andrew, vivien.didelot, davem, idosch, jiri,
rmk+kernel
Hi all,
This patch series supports having VLAN devices on top of DSA/switch
ports while the switch has VLAN filtering globally turned on (as is the
case with Broadcom switches). Whether the switch does global or per-port
VLAN filtering, having VLAN entries for these VLAN devices is
beneficial.
We take care of a few possibly problematic cases:
- adding a VLAN device while there is an existing VLAN entry created by
a VLAN aware bridge. The entire bridge's VLAN database and not just
the specific bridge port is being checked to be safe and conserative
- adding a bridge VLAN entry when there is an existing VLAN device
created is also not possible because that would lead to the bridge
being able to manipulate the VLAN device's VID/attributes under its feet
- enslaving a VLAN device into a VLAN aware bridge since that duplicates
functionality already offered by the VLAN aware bridge
Here are the different test cases that were run to exercise this:
# Create a br0 device with gphy enslaved, verify we can still obtain
# a DHCP lease
ip addr flush dev gphy
ip link add dev br0 type bridge
echo 1 > /sys/class/net/br0/bridge/vlan_filtering
ip link set dev gphy master br0
udhcpc -i br0
# Create a VID 100 interface on top of rgmii_1, verify
# we can ping 192.168.100.1 (the peer)
vconfig add rgmii_1 100
ifconfig rgmii_1.100 192.168.100.10
ping -c 2 192.168.100.1
# Create a VID 42 interface on top of br0 and let it flow tagged
# through the bridge, verify we can ping 192.168.42.1 (the peer)
vconfig add br0 42
bridge vlan add vid 42 dev gphy
bridge vlan add vid 42 dev br0 self
ifconfig br0.42 192.168.42.2
ping -c 2 192.168.42.1
# Delete and re-create rgmii_1.100 and verify things still work
# with or without VLAN filtering applied:
ip link del rgmii_1.100
vconfig add rgmii_1 100
ifconfig rgmii_1.100 192.168.100.10
ping -c 2 192.168.100.1
echo 0 > /sys/class/net/br0/bridge/vlan_filtering
ping -c 2 192.168.100.1
# Delete and attempt to create collision scenarios
ip link del rgmii_1.100
echo 1 > /sys/class/net/br0/bridge/vlan_filtering
# VLAN ID 100 is already claimed by rgmii_1.100
vconfig add rgmii_1 100
brctl addif br0 rgmii_1
# Adding VLAN 100 to rgmii_1 fails since rgmii_1.100 exists
bridge vlan add vid 100 dev rgmii_1
vconfig rem rgmii_1.100
# Adding VLAN 100 to rgmii_1 works since rgmii_1.100 does not exist
bridge vlan add vid 100 dev rgmii_1
# But this fails since we already have a VID with the bridge
vconfig add rgmii_1 100
# Delete and re-create the interface and try to make it enslaved
bridge vlan del vid 100 dev rgmii_1
vconfig add rgmii_1 100
# This fails since the bridge is VLAN aware
brctl addif br0 rgmii_1.100
Florian Fainelli (2):
net: dsa: Deny enslaving VLAN devices into VLAN aware bridge
net: dsa: Add ndo_vlan_rx_{add,kill}_vid implementation
net/dsa/port.c | 12 ++++--
net/dsa/slave.c | 110 +++++++++++++++++++++++++++++++++++++++++++++--
net/dsa/switch.c | 42 ++++++++++++++++++
3 files changed, 157 insertions(+), 7 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 1/2] net: dsa: Deny enslaving VLAN devices into VLAN aware bridge
From: Florian Fainelli @ 2019-02-20 22:35 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, andrew, vivien.didelot, davem, idosch, jiri,
rmk+kernel
In-Reply-To: <20190220223539.22106-1-f.fainelli@gmail.com>
VLAN devices on top of a DSA network device which is already part of a
bridge and with said bridge being VLAN aware should not be allowed to be
enslaved into that bridge. For one, this duplicates functionality
offered by the VLAN aware bridge which supports tagged and untagged VLAN
frames processing and it would make things needlessly complex to e.g.:
propagate FDB/MDB accordingly.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/slave.c | 39 ++++++++++++++++++++++++++++++++++++---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e5e7c04821b..97519ffa445e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1416,16 +1416,49 @@ static int dsa_slave_changeupper(struct net_device *dev,
return err;
}
+static int dsa_slave_upper_vlan_check(struct net_device *dev,
+ struct netdev_notifier_changeupper_info *
+ info)
+{
+ struct netlink_ext_ack *ext_ack;
+ struct net_device *slave;
+ struct dsa_port *dp;
+
+ ext_ack = netdev_notifier_info_to_extack(&info->info);
+
+ if (!is_vlan_dev(dev))
+ return NOTIFY_DONE;
+
+ slave = vlan_dev_real_dev(dev);
+ if (!dsa_slave_dev_check(slave))
+ return NOTIFY_DONE;
+
+ dp = dsa_slave_to_port(slave);
+ if (!dp->bridge_dev)
+ return NOTIFY_DONE;
+
+ /* Deny enslaving a VLAN device into a VLAN-aware bridge */
+ if (br_vlan_enabled(dp->bridge_dev) &&
+ netif_is_bridge_master(info->upper_dev) && info->linking) {
+ NL_SET_ERR_MSG_MOD(ext_ack,
+ "Cannot enslave VLAN device into VLAN aware bridge");
+ return notifier_from_errno(-EINVAL);
+ }
+
+ return NOTIFY_DONE;
+}
+
static int dsa_slave_netdevice_event(struct notifier_block *nb,
unsigned long event, void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
- if (!dsa_slave_dev_check(dev))
- return NOTIFY_DONE;
+ if (event == NETDEV_CHANGEUPPER) {
+ if (!dsa_slave_dev_check(dev))
+ return dsa_slave_upper_vlan_check(dev, ptr);
- if (event == NETDEV_CHANGEUPPER)
return dsa_slave_changeupper(dev, ptr);
+ }
return NOTIFY_DONE;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/2] net: dsa: Add ndo_vlan_rx_{add,kill}_vid implementation
From: Florian Fainelli @ 2019-02-20 22:35 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, andrew, vivien.didelot, davem, idosch, jiri,
rmk+kernel
In-Reply-To: <20190220223539.22106-1-f.fainelli@gmail.com>
In order to properly support VLAN filtering being enabled/disabled on a
bridge, while having other ports being non bridge port members, we need
to support the ndo_vlan_rx_{add,kill}_vid callbacks in order to make
sure the non-bridge ports can continue receiving VLAN tags, even when
the switch is globally configured to do ingress/egress VID checking.
Since we can call dsa_port_vlan_{add,del} with a bridge_dev pointer
NULL, we now need to check that in these two functions.
We specifically deal with two possibly problematic cases:
- creating a bridge VLAN entry while there is an existing VLAN device
claiming that same VID
- creating a VLAN device while there is an existing bridge VLAN entry
with that VID
Those are both resolved with returning -EBUSY back to user-space.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/port.c | 12 ++++++--
net/dsa/slave.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++-
net/dsa/switch.c | 42 ++++++++++++++++++++++++++++
3 files changed, 121 insertions(+), 4 deletions(-)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2d7e01b23572..e3ac9be7a9a0 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -252,7 +252,10 @@ int dsa_port_vlan_add(struct dsa_port *dp,
.vlan = vlan,
};
- if (br_vlan_enabled(dp->bridge_dev))
+ /* Can be called from dsa_slave_port_obj_add() or
+ * dsa_slave_vlan_rx_add_vid()
+ */
+ if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev))
return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
return 0;
@@ -267,10 +270,13 @@ int dsa_port_vlan_del(struct dsa_port *dp,
.vlan = vlan,
};
- if (netif_is_bridge_master(vlan->obj.orig_dev))
+ if (vlan->obj.orig_dev && netif_is_bridge_master(vlan->obj.orig_dev))
return -EOPNOTSUPP;
- if (br_vlan_enabled(dp->bridge_dev))
+ /* Can be called from dsa_slave_port_obj_del() or
+ * dsa_slave_vlan_rx_kill_vid()
+ */
+ if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev))
return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
return 0;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 97519ffa445e..e54f3a7a3457 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -990,6 +990,72 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
return ds->ops->get_ts_info(ds, p->dp->index, ts);
}
+static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
+ u16 vid)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct switchdev_obj_port_vlan vlan = {
+ .vid_begin = vid,
+ .vid_end = vid,
+ /* This API only allows programming tagged, non-PVID VIDs */
+ .flags = 0,
+ };
+ struct bridge_vlan_info info;
+ int ret;
+
+ /* Check for a possible bridge VLAN entry now since there is no
+ * need to emulate the switchdev prepare + commit phase.
+ */
+ if (dp->bridge_dev) {
+ /* br_vlan_get_info() returns -EINVAL or -ENOENT if the
+ * device, respectively the VID is not found, returning
+ * 0 means success, which is a failure for us here.
+ */
+ ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
+ if (ret == 0)
+ return -EBUSY;
+ }
+
+ ret = dsa_port_vlan_add(dp, &vlan, NULL);
+ if (ret == -EOPNOTSUPP)
+ ret = 0;
+
+ return ret;
+}
+
+static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
+ u16 vid)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct switchdev_obj_port_vlan vlan = {
+ .vid_begin = vid,
+ .vid_end = vid,
+ /* This API only allows programming tagged, non-PVID VIDs */
+ .flags = 0,
+ };
+ struct bridge_vlan_info info;
+ int ret;
+
+ /* Check for a possible bridge VLAN entry now since there is no
+ * need to emulate the switchdev prepare + commit phase.
+ */
+ if (dp->bridge_dev) {
+ /* br_vlan_get_info() returns -EINVAL or -ENOENT if the
+ * device, respectively the VID is not found, returning
+ * 0 means success, which is a failure for us here.
+ */
+ ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
+ if (ret == 0)
+ return -EBUSY;
+ }
+
+ ret = dsa_port_vlan_del(dp, &vlan);
+ if (ret == -EOPNOTSUPP)
+ ret = 0;
+
+ return ret;
+}
+
static const struct ethtool_ops dsa_slave_ethtool_ops = {
.get_drvinfo = dsa_slave_get_drvinfo,
.get_regs_len = dsa_slave_get_regs_len,
@@ -1055,6 +1121,8 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
.ndo_setup_tc = dsa_slave_setup_tc,
.ndo_get_stats64 = dsa_slave_get_stats64,
.ndo_get_port_parent_id = dsa_slave_get_port_parent_id,
+ .ndo_vlan_rx_add_vid = dsa_slave_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = dsa_slave_vlan_rx_kill_vid,
};
static const struct switchdev_ops dsa_slave_switchdev_ops = {
@@ -1315,7 +1383,8 @@ int dsa_slave_create(struct dsa_port *port)
if (slave_dev == NULL)
return -ENOMEM;
- slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
+ slave_dev->features = master->vlan_features | NETIF_F_HW_TC |
+ NETIF_F_HW_VLAN_CTAG_FILTER;
slave_dev->hw_features |= NETIF_F_HW_TC;
slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
eth_hw_addr_inherit(slave_dev, master);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 142b294d3446..e1fae969aa73 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -12,6 +12,7 @@
#include <linux/netdevice.h>
#include <linux/notifier.h>
+#include <linux/if_vlan.h>
#include <net/switchdev.h>
#include "dsa_priv.h"
@@ -168,6 +169,43 @@ static int dsa_switch_mdb_del(struct dsa_switch *ds,
return 0;
}
+static int dsa_port_vlan_device_check(struct net_device *vlan_dev,
+ int vlan_dev_vid,
+ void *arg)
+{
+ struct switchdev_obj_port_vlan *vlan = arg;
+ u16 vid;
+
+ for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+ if (vid == vlan_dev_vid)
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static int dsa_port_vlan_check(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
+{
+ const struct dsa_port *dp = dsa_to_port(ds, port);
+ int err = 0;
+
+ /* Device is not bridged, let it proceed with the VLAN device
+ * creation.
+ */
+ if (!dp->bridge_dev)
+ return err;
+
+ /* dsa_slave_vlan_rx_{add,kill}_vid() cannot use the prepare pharse and
+ * already checks whether there is an overlapping bridge VLAN entry
+ * with the same VID, so here we only need to check that if we are
+ * adding a bridge VLAN entry there is not an overlapping VLAN device
+ * claiming that VID.
+ */
+ return vlan_for_each(dp->slave, dsa_port_vlan_device_check,
+ (void *)vlan);
+}
+
static int
dsa_switch_vlan_prepare_bitmap(struct dsa_switch *ds,
const struct switchdev_obj_port_vlan *vlan,
@@ -179,6 +217,10 @@ dsa_switch_vlan_prepare_bitmap(struct dsa_switch *ds,
return -EOPNOTSUPP;
for_each_set_bit(port, bitmap, ds->num_ports) {
+ err = dsa_port_vlan_check(ds, port, vlan);
+ if (err)
+ return err;
+
err = ds->ops->port_vlan_prepare(ds, port, vlan);
if (err)
return err;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 12/12] net: sched: flower: set unlocked flag for flower proto ops
From: Cong Wang @ 2019-02-20 22:36 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <vbf5ztgqe2p.fsf@mellanox.com>
On Tue, Feb 19, 2019 at 2:15 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Mon 18 Feb 2019 at 19:27, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>
> >> Set TCF_PROTO_OPS_DOIT_UNLOCKED for flower classifier to indicate that its
> >> ops callbacks don't require caller to hold rtnl lock.
> >
> > So, if this means RTNL is gone for all cls_flower changes, why
> > do I still see rtnl_lock() in cls_flower.c after all your patches in
> > this set?
>
> It doesn't say that rtnl lock is gone, what it says is that caller
> doesn't have to obtain rtnl lock before calling flower ops callbacks.
So RTNL lock is still a bottle neck after all of these patches,
isn't it? :)
Yeah, please kindly add an explanation for why RTNL lock is still
here.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex
From: Cong Wang @ 2019-02-20 22:43 UTC (permalink / raw)
To: Vlad Buslov
Cc: Ido Schimmel, netdev@vger.kernel.org, jhs@mojatatu.com,
jiri@resnulli.us, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <vbf36okq7rb.fsf@mellanox.com>
On Tue, Feb 19, 2019 at 4:31 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Tue 19 Feb 2019 at 05:26, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Fri, Feb 15, 2019 at 7:35 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>
> >> Another problem that I found in cls_fw and cls_route is that they set
> >> arg->stop when empty. Both of them have code unchanged since it was
> >> committed initially in 2005 so I assume this convention is no longer
> >> relevant because all other classifiers don't do that (they only set
> >> arg->stop when arg->fn returns negative value).
> >>
> >
> > The question is why do you want to use arg->stop==0 as
> > an indication for emptiness? Isn't what arg->count==0
> > supposed to be?
>
> Good question! I initially wanted to implement it like that, but
> reconsidered because iterating through all filters on classifier to
> count them is O(N), and terminating on first filter and relying on
> arg->stop==1 is constant time. Making function that is called
> "tcf_proto_is_empty" linear on number of filters seemed sloppy to me...
Good point, however arg->stop _was_ supposed to set only when
error happens. Probably you want a new arg here to stop on the first
entry.
^ permalink raw reply
* [PATCH] Documentation: networking: switchdev: Update port parent ID section
From: Florian Fainelli @ 2019-02-20 22:58 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, idosch, jiri, Florian Fainelli
Update the section about switchdev drivers having to implement a
switchdev_port_attr_get() function to return
SWITCHDEV_ATTR_ID_PORT_PARENT_ID since that is no longer valid after
commit bccb30254a4a ("net: Get rid of
SWITCHDEV_ATTR_ID_PORT_PARENT_ID").
Fixes: bccb30254a4a ("net: Get rid of SWITCHDEV_ATTR_ID_PORT_PARENT_ID")
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Documentation/networking/switchdev.txt | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index f3244d87512a..ea90243340a9 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -92,11 +92,11 @@ device.
Switch ID
^^^^^^^^^
-The switchdev driver must implement the switchdev op switchdev_port_attr_get
-for SWITCHDEV_ATTR_ID_PORT_PARENT_ID for each port netdev, returning the same
-physical ID for each port of a switch. The ID must be unique between switches
-on the same system. The ID does not need to be unique between switches on
-different systems.
+The switchdev driver must implement the net_device operation
+ndo_get_port_parent_id for each port netdev, returning the same physical ID for
+each port of a switch. The ID must be unique between switches on the same
+system. The ID does not need to be unique between switches on different
+systems.
The switch ID is used to locate ports on a switch and to know if aggregated
ports belong to the same switch.
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex
From: Cong Wang @ 2019-02-20 23:00 UTC (permalink / raw)
To: Vlad Buslov
Cc: Ido Schimmel, netdev@vger.kernel.org, jhs@mojatatu.com,
jiri@resnulli.us, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <vbfzhqrq01r.fsf@mellanox.com>
On Tue, Feb 19, 2019 at 7:20 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Tue 19 Feb 2019 at 05:08, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Fri, Feb 15, 2019 at 2:02 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>
> >> I looked at the code and problem seems to be matchall classifier
> >> specific. My implementation of unlocked cls API assumes that concurrent
> >> insertions are possible and checks for it when deleting "empty" tp.
> >> Since classifiers don't expose number of elements, the only way to test
> >> this is to do tp->walk() on them and assume that walk callback is called
> >> once per filter on every classifier. In your example new tp is created
> >> for second filter, filter insertion fails, number of elements on newly
> >> created tp is checked with tp->walk() before deleting it. However,
> >> matchall classifier always calls the tp->walk() callback once, even when
> >> it doesn't have a valid filter (in this case with NULL filter pointer).
> >
> > Again, this can be eliminated by just switching to normal
> > non-retry logic. This is yet another headache to review this
> > kind of unlock-and-retry logic, I have no idea why you are such
> > a big fan of it.
>
> The retry approach was suggested to me multiple times by Jiri on
> previous code reviews so I assumed it is preferred approach in such
> cases. I don't have a strong preference in this regard, but locking
> whole tp on filter update will remove any parallelism when updating same
> classifier instance concurrently. The goal of these changes is to allow
> parallel rule update and to achieve that I had to introduce some
> complexity into the code.
Yeah, but with unlock-and-retry it would waste more time when
retry occurs. So it can't be better in the worst scenario.
The question is essentially that do we want to waste CPU cycles
when conflicts occurs or just block there until it is safe to enter
the critical section?
And, is the retry bound? Is it possible that we would retry infinitely
as long as we time it correctly?
>
> Now let me explain why these two approaches result completely different
> performance in this case. Lets start with a list of most CPU-consuming
> parts in new filter creation process in descending order (raw data at
> the end of this mail):
>
> 1) Hardware offload - if available and no skip_hw.
> 2) Exts (actions) initalization - most expensive part even with single
> action, CPU usage increases with number of actions per filter.
> 3) cls API.
> 4) Flower classifier data structure initialization.
>
> Note that 1)+2) is ~80% of cost of creating a flower filter. So if we
> just lock the whole flower classifier instance during rule update we
> serialize 1, 2 and 4, and only cls API (~13% of CPU cost) can be
> executed concurrently. However, in proposed flower implementation hw
> offloading and action initialization code is called without any locks
> and tp->lock is only obtained when modifying flower data structures,
> which means that only 3) is serialized and everything else (87% of CPU
> cost) can be executed in parallel.
What about when conflicts detected and retry the whole change?
And, of course, how often do conflicts happen?
Thanks.
^ permalink raw reply
* [PATCH bpf-next v2] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Daniel Borkmann @ 2019-02-20 23:01 UTC (permalink / raw)
To: ast; +Cc: keescook, netdev, Daniel Borkmann
In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
a check was added for BPF_PROG_RUN() that for every invocation preemption is
disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this does
not count for seccomp because only cBPF -> eBPF is loaded here and it does
not make use of any functionality that would require this assertion. Fix this
false positive by adding and using SECCOMP_RUN() variant that does not have
the cant_sleep(); check.
Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Kees Cook <keescook@chromium.org>
---
v1 -> v2:
- More elaborate comment and added SECCOMP_RUN
- Added Kees' ACK from earlier v1 patch
include/linux/filter.h | 22 +++++++++++++++++++++-
kernel/seccomp.c | 2 +-
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f32b3ec..cd7f957 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -533,7 +533,27 @@ struct sk_filter {
struct bpf_prog *prog;
};
-#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
+#define __bpf_prog_run(prog, ctx) \
+ (*(prog)->bpf_func)(ctx, (prog)->insnsi)
+#define __bpf_prog_run__may_preempt(prog, ctx) \
+ ({ __bpf_prog_run(prog, ctx); })
+#define __bpf_prog_run__non_preempt(prog, ctx) \
+ ({ cant_sleep(); __bpf_prog_run(prog, ctx); })
+
+/* Preemption must be disabled when native eBPF programs are run in
+ * order to not break per CPU data structures, for example; make
+ * sure to throw a stack trace under CONFIG_DEBUG_ATOMIC_SLEEP when
+ * we find that preemption is still enabled.
+ *
+ * Only exception today is seccomp, where progs have transitioned
+ * from cBPF to eBPF, and native eBPF is _not_ supported. They can
+ * safely run with preemption enabled.
+ */
+#define BPF_PROG_RUN(prog, ctx) \
+ __bpf_prog_run__non_preempt(prog, ctx)
+
+#define SECCOMP_RUN(prog, ctx) \
+ __bpf_prog_run__may_preempt(prog, ctx)
#define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index e815781..701a3cf 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -268,7 +268,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
* value always takes priority (ignoring the DATA).
*/
for (; f; f = f->prev) {
- u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
+ u32 cur_ret = SECCOMP_RUN(f->prog, sd);
if (ACTION_ONLY(cur_ret) < ACTION_ONLY(ret)) {
ret = cur_ret;
--
2.9.5
^ permalink raw reply related
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