* 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
* Re: [PATCH net-next v4 1/3] net: dsa: add support for bridge flags
From: Florian Fainelli @ 2019-02-20 23:18 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>
> ---
> 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;
There is no struct dsa_switch *ds = dp->ds that is being declared in
dsa_slave_port_attr_get():
net/dsa/slave.c: In function 'dsa_slave_port_attr_get':
net/dsa/slave.c:390:7: error: 'ds' undeclared (first use in this function)
if (ds->ops->port_egress_floods)
^~
net/dsa/slave.c:390:7: note: each undeclared identifier is reported only
once for each function it appears in
scripts/Makefile.build:276: recipe for target 'net/dsa/slave.o' failed
make[4]: *** [net/dsa/slave.o] Error 1
scripts/Makefile.build:492: recipe for target 'net/dsa' failed
make[3]: *** [net/dsa] Error 2
make[3]: *** Waiting for unfinished jobs....
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v4 1/3] net: dsa: add support for bridge flags
From: Russell King - ARM Linux admin @ 2019-02-20 23:23 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, Vivien Didelot, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <6542d026-5c68-56fe-a0c5-47f7a1e2f0fa@gmail.com>
Hi Florian,
Please take over these patches.
Thanks.
On Wed, Feb 20, 2019 at 03:18:44PM -0800, Florian Fainelli wrote:
> 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>
> > ---
> > 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;
>
> There is no struct dsa_switch *ds = dp->ds that is being declared in
> dsa_slave_port_attr_get():
>
> net/dsa/slave.c: In function 'dsa_slave_port_attr_get':
> net/dsa/slave.c:390:7: error: 'ds' undeclared (first use in this function)
> if (ds->ops->port_egress_floods)
> ^~
> net/dsa/slave.c:390:7: note: each undeclared identifier is reported only
> once for each function it appears in
> scripts/Makefile.build:276: recipe for target 'net/dsa/slave.o' failed
> make[4]: *** [net/dsa/slave.o] Error 1
> scripts/Makefile.build:492: recipe for target 'net/dsa' failed
> make[3]: *** [net/dsa] Error 2
> make[3]: *** Waiting for unfinished jobs....
>
> --
> Florian
>
--
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] sysctl: Fix proc_do_large_bitmap for large input buffers
From: Eric Sandeen @ 2019-02-20 23:32 UTC (permalink / raw)
To: Linux Kernel Mailing List, fsdevel, netdev; +Cc: Luis Chamberlain, Kees Cook
Today, proc_do_large_bitmap() truncates a large write input buffer
to PAGE_SIZE - 1, which may result in misparsed numbers at the
(truncated) end of the buffer. Further, it fails to notify the caller
that the buffer was truncated, so it doesn't get called iteratively
to finish the entire input buffer.
Tell the caller if there's more work to do by adding the skipped
amount back to left/*lenp before returning.
To fix the misparsing, reset the position if we have completely
consumed a truncated buffer (or if just one char is left, which
may be a "-" in a range), and ask the caller to come back for
more.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ba4d9e85feb8..970a96659809 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3089,9 +3089,13 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
if (write) {
char *kbuf, *p;
+ size_t skipped = 0;
- if (left > PAGE_SIZE - 1)
+ if (left > PAGE_SIZE - 1) {
left = PAGE_SIZE - 1;
+ /* How much of the buffer we'll skip this pass */
+ skipped = *lenp - left;
+ }
p = kbuf = memdup_user_nul(buffer, left);
if (IS_ERR(kbuf))
@@ -3108,9 +3112,22 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
while (!err && left) {
unsigned long val_a, val_b;
bool neg;
+ size_t saved_left;
+ /* In case we stop parsing mid-number, we can reset */
+ saved_left = left;
err = proc_get_long(&p, &left, &val_a, &neg, tr_a,
sizeof(tr_a), &c);
+ /*
+ * If we consumed the entirety of a truncated buffer or
+ * only one char is left (may be a "-"), then stop here,
+ * reset, & come back for more.
+ */
+ if ((left <= 1) && skipped) {
+ left = saved_left;
+ break;
+ }
+
if (err)
break;
if (val_a >= bitmap_len || neg) {
@@ -3128,6 +3145,15 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
err = proc_get_long(&p, &left, &val_b,
&neg, tr_b, sizeof(tr_b),
&c);
+ /*
+ * If we consumed all of a truncated buffer or
+ * then stop here, reset, & come back for more.
+ */
+ if (!left && skipped) {
+ left = saved_left;
+ break;
+ }
+
if (err)
break;
if (val_b >= bitmap_len || neg ||
@@ -3146,6 +3172,7 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
proc_skip_char(&p, &left, '\n');
}
kfree(kbuf);
+ left += skipped;
} else {
unsigned long bit_a, bit_b = 0;
^ permalink raw reply related
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
From: Eric Sandeen @ 2019-02-20 23:35 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
In-Reply-To: <53be40fc-6ec4-c714-a64e-f69c96f7058f@redhat.com>
Here's a pretty hacky test script to test this code via
ip_local_reserved_ports
-----
#!/bin/bash
# Randomly construct well-formed (sequential, non-overlapping)
# input for ip_local_reserved_ports, feed it to the sysctl,
# then read it back and check for differences.
# Port range to use
PORT_START=1024
PORT_STOP=32768
# Total length of ports string to use
LENGTH=$((4096+$((RANDOM % 16384))))
# String containing our list of ports
PORTS=$PORT_START
# Try 1000 times
for I in `seq 1 1000`; do
# build up the string
while true; do
# Make sure it's discontiguous, skip ahead at least 2
SKIP=$((2 + RANDOM % 10))
PORT_START=$((PORT_START + SKIP))
if [ "$PORT_START" -ge "$PORT_STOP" ]; then
break;
fi
# 14856-14863,14861
# Add a range, or a single port
USERANGE=$((RANDOM % 2))
if [ "$USERANGE" -eq "1" ]; then
RANGE_START=$PORT_START
RANGE_LEN=$((1 + RANDOM % 10))
RANGE_END=$((RANGE_START + RANGE_LEN))
PORTS="${PORTS},${RANGE_START}-${RANGE_END}"
# Break out if we've done enough
if [ "$RANGE_END" -eq "$PORT_STOP" ]; then
break;
fi
PORT_START=$RANGE_END
else
PORTS="${PORTS},${PORT_START}"
fi
if [ "${#PORTS}" -gt "$LENGTH" ]; then
break;
fi
done
# See if we get out what we put in
echo "Trial $I"
echo $PORTS > port_list
cat port_list > /proc/sys/net/ipv4/ip_local_reserved_ports || break
cat /proc/sys/net/ipv4/ip_local_reserved_ports > port_list_out
diff -uq port_list port_list_out || break
done
^ permalink raw reply
* [PATCH net-next v5 0/3] net: dsa: mv88e6xxx: fix IPv6
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, andrew, vivien.didelot, davem, rmk+kernel,
hkallweit1
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.
Russell King (3):
net: dsa: add support for bridge flags
net: dsa: mv88e6xxx: add support for bridge flags
net: dsa: enable flooding for bridge ports
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 | 9 +++++++++
5 files changed, 60 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
v5: fix build failure in patch #1
--
2.17.1
^ permalink raw reply
* [PATCH net-next v5 1/3] net: dsa: add support for bridge flags
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
To: netdev
Cc: Russell King, Florian Fainelli, andrew, vivien.didelot, davem,
hkallweit1
In-Reply-To: <20190220233506.22210-1-f.fainelli@gmail.com>
From: Russell King <rmk+kernel@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>
[florian: Add missing dp and ds variables declaration to fix build]
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
include/net/dsa.h | 2 ++
net/dsa/dsa_priv.h | 2 ++
net/dsa/port.c | 17 +++++++++++++++++
net/dsa/slave.c | 9 +++++++++
4 files changed, 30 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..85dc68611002 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;
@@ -381,9 +384,15 @@ static int dsa_slave_get_port_parent_id(struct net_device *dev,
static int dsa_slave_port_attr_get(struct net_device *dev,
struct switchdev_attr *attr)
{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
+
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.17.1
^ permalink raw reply related
* [PATCH net-next v5 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
To: netdev; +Cc: Russell King, andrew, vivien.didelot, davem, hkallweit1
In-Reply-To: <20190220233506.22210-1-f.fainelli@gmail.com>
From: Russell King <rmk+kernel@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
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
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.17.1
^ permalink raw reply related
* [PATCH net-next v5 3/3] net: dsa: enable flooding for bridge ports
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
To: netdev; +Cc: Russell King, andrew, vivien.didelot, davem, hkallweit1
In-Reply-To: <20190220233506.22210-1-f.fainelli@gmail.com>
From: Russell King <rmk+kernel@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.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
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.17.1
^ permalink raw reply related
* Re: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Joe Perches @ 2019-02-20 23:49 UTC (permalink / raw)
To: Florian Fainelli, Tristram.Ha, andrew
Cc: sergio.paracuellos, pavel, UNGLinuxDriver, netdev
In-Reply-To: <BC575D76-0419-4318-B924-81E29858B8E9@gmail.com>
On Tue, 2019-02-12 at 19:51 -0800, Florian Fainelli wrote:
> On February 12, 2019 6:39:49 PM PST, Tristram.Ha@microchip.com wrote:
> > > > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port,
> > > > + bool freeze)
> > > > +{
> > > > + struct ksz_port *p = &dev->ports[port];
> > > > + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
> > >
> > > Reverse Christmas tree.
> >
> > There was a checkpatch.pl patch in 2016 that tried to check this, but
> > it was never accepted?
https://lore.kernel.org/patchwork/patch/732076/
While I still think use of reverse christmas tree is misguided,
there were some disagreements about what form it really is:
Is this reverse christmas tree?
void foo(void)
{
int aa;
int a = 1;
or is
void foo(void)
{
int a = 1;
int aa;
IMHO: neither really helps to visually find or scan for
automatics so the whole concept isn't particularly useful.
^ permalink raw reply
* Re: mlx5 stable backport help
From: Saeed Mahameed @ 2019-02-20 23:58 UTC (permalink / raw)
To: davem@davemloft.net; +Cc: netdev@vger.kernel.org, Or Gerlitz
In-Reply-To: <20190220.122740.445688569983151429.davem@davemloft.net>
On Wed, 2019-02-20 at 12:27 -0800, David Miller wrote:
> From: Saeed Mahameed <saeedm@mellanox.com>
> Date: Wed, 20 Feb 2019 20:01:00 +0000
>
> > On Tue, 2019-02-19 at 22:36 -0800, David Miller wrote:
> > > 218d05ce326f9e1b40a56085431fa1068b43d5d9 ("net/mlx5e: Don't
> > > overwrite
> > > pedit action when multiple pedit used")
> > >
> >
> > I tried and this patch applies smoothly on 4.19.
>
> Does not apply to v4.19.24 which is the current stable release.
>
> [davem@localhost linux-stable]$ git am QUEUE/mlx5e/0002-net-mlx5e-
> Don-t-overwrite-pedit-action-when-multiple.patch
> Applying: net/mlx5e: Don't overwrite pedit action when multiple pedit
> used
> error: patch failed:
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:128
> error: drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: patch does
> not apply
> Patch failed at 0001 net/mlx5e: Don't overwrite pedit action when
> multiple pedit used
> hint: Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --
> abort".
Strange, worked for me: (I saw some warnings though!)
saeedm@sx1[linux](stbl/4.19)$ git cherry-pick
218d05ce326f9e1b40a56085431fa1068b43d5d9
warning: inexact rename detection was skipped due to too many files.
warning: you may want to set your merge.renamelimit variable to at
least 2119 and retry the command.
[stbl/4.19 efa42113bb1d] net/mlx5e: Don't overwrite pedit action when
multiple pedit used
Author: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Date: Mon Jan 28 15:28:06 2019 -0800
1 file changed, 15 insertions(+), 10 deletions(-)
saeedm@sx1[linux](stbl/4.19)$ git log --oneline
efa42113bb1d (HEAD -> stbl/4.19) net/mlx5e: Don't overwrite pedit
action when multiple pedit used
f287634fe321 (tag: v4.19.24, stable/linux-4.19.y) Linux 4.19.24
dd5f4d067a2c mm: proc: smaps_rollup: fix pss_locked calculation
I did what the git suggested and:
$ git config merge.renamelimit 2119
Tried again, it worked with now warnings.
Maybe you have some git config which fails on the warning i saw..
Thanks,
Saeed.
^ permalink raw reply
* Re: [PATCH bpf-next v2] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Alexei Starovoitov @ 2019-02-20 23:59 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, keescook, netdev
In-Reply-To: <20190220230135.9748-1-daniel@iogearbox.net>
On Thu, Feb 21, 2019 at 12:01:35AM +0100, Daniel Borkmann wrote:
> 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>
Applied, Thanks
^ permalink raw reply
* [PATCH net 0/2] bnxt_en: firmware message delay fixes.
From: Michael Chan @ 2019-02-21 0:07 UTC (permalink / raw)
To: davem; +Cc: netdev
We were seeing some intermittent firmware message timeouts in our lab and
these 2 small patches fix them. Please apply to stable as well. Thanks.
Michael Chan (2):
bnxt_en: Fix typo in firmware message timeout logic.
bnxt_en: Wait longer for the firmware message response to complete.
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
--
2.5.1
^ permalink raw reply
* [PATCH net 1/2] bnxt_en: Fix typo in firmware message timeout logic.
From: Michael Chan @ 2019-02-21 0:07 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1550707652-14747-1-git-send-email-michael.chan@broadcom.com>
The logic that polls for the firmware message response uses a shorter
sleep interval for the first few passes. But there was a typo so it
was using the wrong counter (larger counter) for these short sleep
passes. The result is a slightly shorter timeout period for these
firmware messages than intended. Fix it by using the proper counter.
Fixes: 9751e8e71487 ("bnxt_en: reduce timeout on initial HWRM calls")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8bc7e49..1ddd672 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3903,7 +3903,7 @@ static int bnxt_hwrm_do_send_msg(struct bnxt *bp, void *msg, u32 msg_len,
if (len)
break;
/* on first few passes, just barely sleep */
- if (i < DFLT_HWRM_CMD_TIMEOUT)
+ if (i < HWRM_SHORT_TIMEOUT_COUNTER)
usleep_range(HWRM_SHORT_MIN_TIMEOUT,
HWRM_SHORT_MAX_TIMEOUT);
else
--
2.5.1
^ 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