* Re: [PATCH net-next 00/13] liquidio: Tx queue cleanup
From: David Miller @ 2018-03-25 20:22 UTC (permalink / raw)
To: felix.manlunas
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
intiyaz.basha
In-Reply-To: <20180324003618.GA6457@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Fri, 23 Mar 2018 17:36:18 -0700
> From: Intiyaz Basha <intiyaz.basha@cavium.com>
>
> Moved some common function to octeon_network.h
> Removed some unwanted functions and checks.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink
From: Jakub Kicinski @ 2018-03-25 19:53 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, roopa, shm, jiri, idosch, David Ahern
In-Reply-To: <2c80d538-6881-709a-58b9-e0dfb669fac3@cumulusnetworks.com>
On Sun, 25 Mar 2018 08:27:42 -0600, David Ahern wrote:
> On 3/25/18 12:35 AM, Jakub Kicinski wrote:
> > On Sat, 24 Mar 2018 09:02:45 -0600, David Ahern wrote:
> >>>> diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
> >>>> index 09388c06171d..449b2a1a1800 100644
> >>>> --- a/drivers/net/netdevsim/Makefile
> >>>> +++ b/drivers/net/netdevsim/Makefile
> >>>> @@ -9,3 +9,7 @@ ifeq ($(CONFIG_BPF_SYSCALL),y)
> >>>> netdevsim-objs += \
> >>>> bpf.o
> >>>> endif
> >>>> +
> >>>> +ifneq ($(CONFIG_NET_DEVLINK),)
> >>>
> >>> Hm. Don't you need MAY_USE_DEVLINK dependency perhaps?
> >>
> >> mlxsw uses CONFIG_NET_DEVLINK in its Makefile.
> >>
> >> MAY_USE_DEVLINK seems to only be used in Kconfig files. Not clear to me
> >> why it is needed at all.
> >
> > NETDEVSIM=y && DEVLINK=m
> >
>
> ok. I purposely did not add DEVLINK as a dependency to netdevsim to make
> the resource controller truly optional. Can add it if you prefer.
Oh, no, I don't mind. I just thought NETDEVSIM=y DEVLINK=m case will
break the build, but I haven't tested. If it works perfect, let's not
add unnecessary dependencies :)
(FWIW the MAY_USE_DEVLINK dep is basically depend on DEVLINK ||
DEVLINK=n, so one can still build without devlink but if devlink is a
module netdevsim will also have to be.)
^ permalink raw reply
* [PATCH net] team: move dev_mc_sync after master_upper_dev_link in team_port_add
From: Xin Long @ 2018-03-25 17:25 UTC (permalink / raw)
To: network dev; +Cc: davem, Jiri Pirko
The same fix as in 'bonding: move dev_mc_sync after master_upper_dev_link
in bond_enslave' is needed for team driver.
The panic can be reproduced easily:
ip link add team1 type team
ip link set team1 up
ip link add link team1 vlan1 type vlan id 80
ip link set vlan1 master team1
Fixes: cb41c997d444 ("team: team should sync the port's uc/mc addrs when add a port")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
drivers/net/team/team.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 56c701b..befed2d 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1197,11 +1197,6 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
goto err_dev_open;
}
- netif_addr_lock_bh(dev);
- dev_uc_sync_multiple(port_dev, dev);
- dev_mc_sync_multiple(port_dev, dev);
- netif_addr_unlock_bh(dev);
-
err = vlan_vids_add_by_dev(port_dev, dev);
if (err) {
netdev_err(dev, "Failed to add vlan ids to device %s\n",
@@ -1241,6 +1236,11 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
goto err_option_port_add;
}
+ netif_addr_lock_bh(dev);
+ dev_uc_sync_multiple(port_dev, dev);
+ dev_mc_sync_multiple(port_dev, dev);
+ netif_addr_unlock_bh(dev);
+
port->index = -1;
list_add_tail_rcu(&port->list, &team->port_list);
team_port_enable(team, port);
@@ -1265,8 +1265,6 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
vlan_vids_del_by_dev(port_dev, dev);
err_vids_add:
- dev_uc_unsync(port_dev, dev);
- dev_mc_unsync(port_dev, dev);
dev_close(port_dev);
err_dev_open:
--
2.1.0
^ permalink raw reply related
* [PATCH net 3/3] bonding: process the err returned by dev_set_allmulti properly in bond_enslave
From: Xin Long @ 2018-03-25 17:16 UTC (permalink / raw)
To: network dev
Cc: davem, Jiri Pirko, Wang Chen, Veaceslav Falico,
Nikolay Aleksandrov
In-Reply-To: <cover.1521997984.git.lucien.xin@gmail.com>
When dev_set_promiscuity(1) succeeds but dev_set_allmulti(1) fails,
dev_set_promiscuity(-1) should be done before going to the err path.
Otherwise, dev->promiscuity will leak.
Fixes: 7e1a1ac1fbaa ("bonding: Check return of dev_set_promiscuity/allmulti")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
drivers/net/bonding/bond_main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 55e1985..b7b1130 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1706,8 +1706,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
/* set allmulti level to new slave */
if (bond_dev->flags & IFF_ALLMULTI) {
res = dev_set_allmulti(slave_dev, 1);
- if (res)
+ if (res) {
+ if (bond_dev->flags & IFF_PROMISC)
+ dev_set_promiscuity(slave_dev, -1);
goto err_sysfs_del;
+ }
}
netif_addr_lock_bh(bond_dev);
--
2.1.0
^ permalink raw reply related
* [PATCH net 2/3] bonding: move dev_mc_sync after master_upper_dev_link in bond_enslave
From: Xin Long @ 2018-03-25 17:16 UTC (permalink / raw)
To: network dev
Cc: davem, Jiri Pirko, Wang Chen, Veaceslav Falico,
Nikolay Aleksandrov
In-Reply-To: <cover.1521997984.git.lucien.xin@gmail.com>
Beniamino found a crash when adding vlan as slave of bond which is also
the parent link:
ip link add bond1 type bond
ip link set bond1 up
ip link add link bond1 vlan1 type vlan id 80
ip link set vlan1 master bond1
The call trace is as below:
[<ffffffffa850842a>] queued_spin_lock_slowpath+0xb/0xf
[<ffffffffa8515680>] _raw_spin_lock+0x20/0x30
[<ffffffffa83f6f07>] dev_mc_sync+0x37/0x80
[<ffffffffc08687dc>] vlan_dev_set_rx_mode+0x1c/0x30 [8021q]
[<ffffffffa83efd2a>] __dev_set_rx_mode+0x5a/0xa0
[<ffffffffa83f7138>] dev_mc_sync_multiple+0x78/0x80
[<ffffffffc084127c>] bond_enslave+0x67c/0x1190 [bonding]
[<ffffffffa8401909>] do_setlink+0x9c9/0xe50
[<ffffffffa8403bf2>] rtnl_newlink+0x522/0x880
[<ffffffffa8403ff7>] rtnetlink_rcv_msg+0xa7/0x260
[<ffffffffa8424ecb>] netlink_rcv_skb+0xab/0xc0
[<ffffffffa83fe498>] rtnetlink_rcv+0x28/0x30
[<ffffffffa8424850>] netlink_unicast+0x170/0x210
[<ffffffffa8424bf8>] netlink_sendmsg+0x308/0x420
[<ffffffffa83cc396>] sock_sendmsg+0xb6/0xf0
This is actually a dead lock caused by sync slave hwaddr from master when
the master is the slave's 'slave'. This dead loop check is actually done
by netdev_master_upper_dev_link. However, Commit 1f718f0f4f97 ("bonding:
populate neighbour's private on enslave") moved it after dev_mc_sync.
This patch is to fix it by moving dev_mc_sync after master_upper_dev_link,
so that this loop check would be earlier than dev_mc_sync. It also moves
if (mode == BOND_MODE_8023AD) into if (!bond_uses_primary) clause as an
improvement.
Note team driver also has this issue, I will fix it in another patch.
Fixes: 1f718f0f4f97 ("bonding: populate neighbour's private on enslave")
Reported-by: Beniamino Galvani <bgalvani@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
drivers/net/bonding/bond_main.c | 73 ++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 38 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0c299de..55e1985 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1528,44 +1528,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
goto err_close;
}
- /* If the mode uses primary, then the following is handled by
- * bond_change_active_slave().
- */
- if (!bond_uses_primary(bond)) {
- /* set promiscuity level to new slave */
- if (bond_dev->flags & IFF_PROMISC) {
- res = dev_set_promiscuity(slave_dev, 1);
- if (res)
- goto err_close;
- }
-
- /* set allmulti level to new slave */
- if (bond_dev->flags & IFF_ALLMULTI) {
- res = dev_set_allmulti(slave_dev, 1);
- if (res)
- goto err_close;
- }
-
- netif_addr_lock_bh(bond_dev);
-
- dev_mc_sync_multiple(slave_dev, bond_dev);
- dev_uc_sync_multiple(slave_dev, bond_dev);
-
- netif_addr_unlock_bh(bond_dev);
- }
-
- if (BOND_MODE(bond) == BOND_MODE_8023AD) {
- /* add lacpdu mc addr to mc list */
- u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
-
- dev_mc_add(slave_dev, lacpdu_multicast);
- }
-
res = vlan_vids_add_by_dev(slave_dev, bond_dev);
if (res) {
netdev_err(bond_dev, "Couldn't add bond vlan ids to %s\n",
slave_dev->name);
- goto err_hwaddr_unsync;
+ goto err_close;
}
prev_slave = bond_last_slave(bond);
@@ -1725,6 +1692,37 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
goto err_upper_unlink;
}
+ /* If the mode uses primary, then the following is handled by
+ * bond_change_active_slave().
+ */
+ if (!bond_uses_primary(bond)) {
+ /* set promiscuity level to new slave */
+ if (bond_dev->flags & IFF_PROMISC) {
+ res = dev_set_promiscuity(slave_dev, 1);
+ if (res)
+ goto err_sysfs_del;
+ }
+
+ /* set allmulti level to new slave */
+ if (bond_dev->flags & IFF_ALLMULTI) {
+ res = dev_set_allmulti(slave_dev, 1);
+ if (res)
+ goto err_sysfs_del;
+ }
+
+ netif_addr_lock_bh(bond_dev);
+ dev_mc_sync_multiple(slave_dev, bond_dev);
+ dev_uc_sync_multiple(slave_dev, bond_dev);
+ netif_addr_unlock_bh(bond_dev);
+
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ /* add lacpdu mc addr to mc list */
+ u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
+
+ dev_mc_add(slave_dev, lacpdu_multicast);
+ }
+ }
+
bond->slave_cnt++;
bond_compute_features(bond);
bond_set_carrier(bond);
@@ -1748,6 +1746,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
return 0;
/* Undo stages on error */
+err_sysfs_del:
+ bond_sysfs_slave_del(new_slave);
+
err_upper_unlink:
bond_upper_dev_unlink(bond, new_slave);
@@ -1768,10 +1769,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
synchronize_rcu();
slave_disable_netpoll(new_slave);
-err_hwaddr_unsync:
- if (!bond_uses_primary(bond))
- bond_hw_addr_flush(bond_dev, slave_dev);
-
err_close:
slave_dev->priv_flags &= ~IFF_BONDING;
dev_close(slave_dev);
--
2.1.0
^ permalink raw reply related
* [PATCH net 1/3] bonding: fix the err path for dev hwaddr sync in bond_enslave
From: Xin Long @ 2018-03-25 17:16 UTC (permalink / raw)
To: network dev
Cc: davem, Jiri Pirko, Wang Chen, Veaceslav Falico,
Nikolay Aleksandrov
In-Reply-To: <cover.1521997984.git.lucien.xin@gmail.com>
vlan_vids_add_by_dev is called right after dev hwaddr sync, so on
the err path it should unsync dev hwaddr. Otherwise, the slave
dev's hwaddr will never be unsync when this err happens.
Fixes: 1ff412ad7714 ("bonding: change the bond's vlan syncing functions with the standard ones")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
drivers/net/bonding/bond_main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c669554..0c299de 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1565,7 +1565,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
if (res) {
netdev_err(bond_dev, "Couldn't add bond vlan ids to %s\n",
slave_dev->name);
- goto err_close;
+ goto err_hwaddr_unsync;
}
prev_slave = bond_last_slave(bond);
@@ -1755,9 +1755,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
netdev_rx_handler_unregister(slave_dev);
err_detach:
- if (!bond_uses_primary(bond))
- bond_hw_addr_flush(bond_dev, slave_dev);
-
vlan_vids_del_by_dev(slave_dev, bond_dev);
if (rcu_access_pointer(bond->primary_slave) == new_slave)
RCU_INIT_POINTER(bond->primary_slave, NULL);
@@ -1771,6 +1768,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
synchronize_rcu();
slave_disable_netpoll(new_slave);
+err_hwaddr_unsync:
+ if (!bond_uses_primary(bond))
+ bond_hw_addr_flush(bond_dev, slave_dev);
+
err_close:
slave_dev->priv_flags &= ~IFF_BONDING;
dev_close(slave_dev);
--
2.1.0
^ permalink raw reply related
* [PATCH net 0/3] bonding: a bunch of fixes for dev hwaddr sync in bond_enslave
From: Xin Long @ 2018-03-25 17:16 UTC (permalink / raw)
To: network dev
Cc: davem, Jiri Pirko, Wang Chen, Veaceslav Falico,
Nikolay Aleksandrov
This patchset is mainly to fix a crash when adding vlan as slave of
bond which is also the parent link in patch 2/3, and also fix some
err process problems in bond_enslave in patch 1/3 and 3/3.
Xin Long (3):
bonding: fix the err path for dev hwaddr sync in bond_enslave
bonding: move dev_mc_sync after master_upper_dev_link in bond_enslave
bonding: process the err returned by dev_set_allmulti properly in
bond_enslave
drivers/net/bonding/bond_main.c | 73 +++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 36 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: [net-next 03/15] net/mlx5e: PFC stall prevention support
From: Andrew Lunn @ 2018-03-25 16:18 UTC (permalink / raw)
To: Gal Pressman; +Cc: Saeed Mahameed, David S. Miller, netdev, Inbar Karmy
In-Reply-To: <220ed51e-5aeb-1a82-6c43-749e9cf8c4c1@mellanox.com>
> > Shouldn't you map a value of MLX5E_PFC_PREVEN_AUTO_TOUT_MSEC back to
> > PFC_STORM_PREVENTION_AUTO?
>
> We discussed this point internally, mapping MLX5E_PFC_PREVEN_AUTO_TOUT_MSEC (100) to
> PFC_STORM_PREVENTION_AUTO might cause confusion when the user explicitly asks for 100msec timeout
> and gets auto in his following query.
> Also, this way the "auto" timeout is visible to the user, which might help him get an initial
> clue of which values are recommended.
Yes, this is a fair point, which is why i asked the question. Either
way, it can cause confusion. 'I configured it to auto, but it always
returns 100, not auto.'
Whatever is decided, it should be consistent across drivers. So please
add some documentation to the ethtool header file about what is
expected.
Andrew
^ permalink raw reply
* Re: [PATCH net-next RFC V1 5/5] net: mdio: Add a driver for InES time stamping IP core.
From: Andrew Lunn @ 2018-03-25 15:59 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, devicetree, David Miller, Florian Fainelli, Mark Rutland,
Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20180325045151.kq7mjopjwzo6w2vw@localhost>
On Sat, Mar 24, 2018 at 09:51:52PM -0700, Richard Cochran wrote:
> On Sat, Mar 24, 2018 at 07:48:58PM +0100, Andrew Lunn wrote:
> > As far as i can see, you have three basic problems:
> >
> > 1) How do you associate the PTP device to the netdev?
> > 2) How do you get the information you need to configure the PTP device
>
> Yes, yes.
>
> > 3) How do you limit the MAC/PHY to what the PTP device can do.
>
> Hm, I don't think this is important.
So you are happy that the PTP device will cause the MC/PHY link to
break down when it is asked to do something it cannot do? Take for
example a Marvell MAC connected to a Marvell PHY doing 2.5Gbps SGMII
because it can. But say the PTP device cannot be clocked that fast,
and the MII links break.... You as a PTP maintainer might be happy
with that, but as a PHY maintainer, i'm not too happy with this.
> Right, so phylib can operate on phydev->attached_dev->mdiots;
So first off, it is not an MDIO device. You current code is a horrible
hack which gets a NACK. Use a phandle, and have
of_mdiobus_register_phy() follow the phandle to get the device.
To keep lifecycle issues simple, i would also keep it in phydev, not
netdev.
mdiots as a name is completely wrong. It is not an MDIO device. Maybe
in the future some devices will be MDIO, or I2C, or SPI. Just call it
ptpdev. This ptpdev needs to be control bus agnostic. You need a
ptpdev core API exposing functions like ptpdev_hwtstamp,
ptpdev_rxtstamp, ptpdev_txtstamp, ptpdev_link_change, which take a
ptpdev. Have phy_link_change call ptpdev_link_change. You have the
flexibility in that if in the future you do care that your ptpdev
breaks the MAC-PHY link, you can add a ptpdev_validate_advertise,
which allows the ptpdev to mask out link modes it does not support.
Your ptp device, when probing needs to register with the ptpdev core,
passing a generic ptpdev_ops for the operations its support. How it
talks to the device, MMIO, SPI, I2C is hidden within the device
driver.
You can then clean up the code in timestamping.c. Code like:
phydev = skb->dev->phydev;
if (likely(phydev->drv->txtstamp)) {
clone = skb_clone_sk(skb);
if (!clone)
return;
phydev->drv->txtstamp(phydev, clone, type);
}
violates the layering, and the locking looks broken. Add a
phy_txtstamp() call to phylib. It can then either call into the PHY
driver, or use the call the ptpdev API, or -EOPNOTSUP.
Andrew
^ permalink raw reply
* Hello Beautiful
From: Jack @ 2018-03-25 15:49 UTC (permalink / raw)
Hi Dear, my name is Jack and i am seeking for a relationship in which i will feel loved after a series of failed relationships.
I am hoping that you would be interested and we could possibly get to know each other more if you do not mind. I am open to answering questions from you as i think my approach is a little inappropriate. Hope to hear back from you.
Jack.
^ permalink raw reply
* Re: [PATCH RFC net-next 1/7] net: Fix fib notifer to return errno
From: Ido Schimmel @ 2018-03-25 15:37 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski
In-Reply-To: <e2115470-778a-9923-35a3-04e5d9c8af91@cumulusnetworks.com>
On Sun, Mar 25, 2018 at 08:00:19AM -0600, David Ahern wrote:
> On 3/25/18 2:16 AM, Ido Schimmel wrote:
> > On Thu, Mar 22, 2018 at 03:57:51PM -0700, David Ahern wrote:
> >> Notifier handlers use notifier_from_errno to convert any potential error
> >> to an encoded format. As a consequence the other side, call_fib_notifiers
> >> in this case, needs to use notifier_to_errno to return the error from
> >> the handler back to its caller.
> >>
> >> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> >> ---
> >> net/core/fib_notifier.c | 5 ++++-
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
> >> index 5ace0705a3f9..14ba52ebe8c9 100644
> >> --- a/net/core/fib_notifier.c
> >> +++ b/net/core/fib_notifier.c
> >> @@ -21,8 +21,11 @@ EXPORT_SYMBOL(call_fib_notifier);
> >> int call_fib_notifiers(struct net *net, enum fib_event_type event_type,
> >> struct fib_notifier_info *info)
> >
> > There's another (less interesting case) - call_fib_notifier(), which is
> > used to dump the FIB tables for the caller registering to the
> > notification chain.
> >
> > For example, if you have a non-default FIB rule in the system and you
> > modprobe mlxsw, you'll get a silent failure and routes will not be
> > offloaded. On the other hand, I'm not sure we want to fail the module
> > loading in such cases.
>
> right. In normal cases the driver is loaded to create the netdevices
> long before any networking config is done. So it seems to me the use
> case you refer to, some user would have go out of there way to create a
> situation where they install config that is not supported by the driver.
Yes.
> > A possible solution is to have the driver emit a warning via extack for
> > each route/rule being notified after the abort mechanism was triggered.
>
> extack is not available on module load.
I'm aware. I meant that during module load we'll trigger the abort
mechanism (due to an unsupported FIB rule for example), then when user
configures additional routes and extack is available we'll emit a
warning.
> Per past discussions, something you suggested, we need a message for
> "out-of-line" cases like this where a driver notifies userspace of a
> problem.
That's another possibility. We can implement both options to make it
perfectly clear to users and daemons what's going on.
^ permalink raw reply
* Re: [PATCH RFC v2 net-next 00/21] net/ipv6: Separate data structures for FIB and data path
From: David Ahern @ 2018-03-25 15:09 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, roopa, eric.dumazet, weiwan, kafai, yoshfuji
In-Reply-To: <20180324155957.GA15120@splinter>
On 3/24/18 9:59 AM, Ido Schimmel wrote:
>> As you know, my preference is to move to nexthop objects (makes fib6_nh
>> optional). I have IPv4 done; IPv6 requires this patch set.
>
> After going over your presentation [1] I was under the impression that
> the fib6_info will be optional, not fib6_nh: "Idea is similar to adding
> id to fib_info that is exposed to userspace. Subsequent routes pass id
> to avoid fib_info overhead".
Just using that as an analogy to explain the idea in terms of something
that already exists.
>
> But I think misunderstood you. You want to introduce the nexthop API
> that will allow you to have multiple fib6_info pointing to the same
> fib6_nh?
>
> 1. http://vger.kernel.org/netconf2017_files/nexthop-objects.pdf
>
I see nexthop specs as device, gateway, lwtunnel_state and flags. That's
the basic building block. A nexthop group is multiple nexthops where
each nexthop in the group as its own weight.
The fib_info struct has more than that -- data unrelated to a netxthop
and is really a next level struct.
^ permalink raw reply
* Re: [PATCH RFC v2 net-next 19/21] net/ipv6: separate handling of FIB entries from dst based routes
From: David Ahern @ 2018-03-25 14:49 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, roopa, eric.dumazet, weiwan, kafai, yoshfuji
In-Reply-To: <20180324160209.GB15120@splinter>
On 3/24/18 10:02 AM, Ido Schimmel wrote:
>>
>> ok, I'll take a look. I thought I verified both paths (fib6_info and
>> dst) were freeing the metrics.
>
> I get this from kmemleak (applied your patchset on top of fe2d55d295cf):
>
> unreferenced object 0xffff88004e2c16c8 (size 96):
> comm "systemd-network", pid 1255, jiffies 4295166424 (age 957.858s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> ip6_route_info_create (/net/ipv6/route.c:2849)
> ip6_route_add (/net/ipv6/route.c:2975)
> inet6_rtm_newroute (/net/ipv6/route.c:4357)
> rtnetlink_rcv_msg (/net/core/rtnetlink.c:4643)
> netlink_rcv_skb (/net/netlink/af_netlink.c:2445)
> netlink_unicast (/net/netlink/af_netlink.c:1309 /net/netlink/af_netlink.c:1334)
> netlink_sendmsg (/net/netlink/af_netlink.c:1897)
> sock_sendmsg (/net/socket.c:630 /net/socket.c:639)
> SYSC_sendto (/net/socket.c:1748)
> do_syscall_64 (/arch/x86/entry/common.c:287)
> entry_SYSCALL_64_after_hwframe (/arch/x86/entry/entry_64.S:239)
> 0xffffffffffffffff (/./include/asm-generic/sections.h:42)
>
Thanks for confirming. I'll take care of it.
^ permalink raw reply
* [PATCH v7 4/7] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Ariel Elior,
everest-linux-l2, linux-kernel
In-Reply-To: <1521988761-30344-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing
the register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 12 ++++++++----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 2 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 4 +++-
6 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 352beff..d847e1b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -166,6 +166,12 @@ do { \
#define REG_RD8(bp, offset) readb(REG_ADDR(bp, offset))
#define REG_RD16(bp, offset) readw(REG_ADDR(bp, offset))
+#define REG_WR_RELAXED(bp, offset, val) \
+ writel_relaxed((u32)val, REG_ADDR(bp, offset))
+
+#define REG_WR16_RELAXED(bp, offset, val) \
+ writew_relaxed((u16)val, REG_ADDR(bp, offset))
+
#define REG_WR(bp, offset, val) writel((u32)val, REG_ADDR(bp, offset))
#define REG_WR8(bp, offset, val) writeb((u8)val, REG_ADDR(bp, offset))
#define REG_WR16(bp, offset, val) writew((u16)val, REG_ADDR(bp, offset))
@@ -758,10 +764,8 @@ struct bnx2x_fastpath {
#if (BNX2X_DB_SHIFT < BNX2X_DB_MIN_SHIFT)
#error "Min DB doorbell stride is 8"
#endif
-#define DOORBELL(bp, cid, val) \
- do { \
- writel((u32)(val), bp->doorbells + (bp->db_size * (cid))); \
- } while (0)
+#define DOORBELL_RELAXED(bp, cid, val) \
+ writel_relaxed((u32)(val), (bp)->doorbells + ((bp)->db_size * (cid)))
/* TX CSUM helpers */
#define SKB_CS_OFF(skb) (offsetof(struct tcphdr, check) - \
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 0f86f18..95871576 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4156,7 +4156,7 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
/* make sure descriptor update is observed by HW */
wmb();
- DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
+ DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
mmiowb();
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
index a5265e1..a8ce5c5 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
@@ -522,8 +522,8 @@ static inline void bnx2x_update_rx_prod(struct bnx2x *bp,
wmb();
for (i = 0; i < sizeof(rx_prods)/4; i++)
- REG_WR(bp, fp->ustorm_rx_prods_offset + i*4,
- ((u32 *)&rx_prods)[i]);
+ REG_WR_RELAXED(bp, fp->ustorm_rx_prods_offset + i * 4,
+ ((u32 *)&rx_prods)[i]);
mmiowb(); /* keep prod updates ordered */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 39af4f8..da18aa2 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -2593,7 +2593,7 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode)
txdata->tx_db.data.prod += 2;
/* make sure descriptor update is observed by the HW */
wmb();
- DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
+ DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
mmiowb();
barrier();
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 74fc9af..146c40d 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -3817,8 +3817,8 @@ static void bnx2x_sp_prod_update(struct bnx2x *bp)
*/
mb();
- REG_WR16(bp, BAR_XSTRORM_INTMEM + XSTORM_SPQ_PROD_OFFSET(func),
- bp->spq_prod_idx);
+ REG_WR16_RELAXED(bp, BAR_XSTRORM_INTMEM + XSTORM_SPQ_PROD_OFFSET(func),
+ bp->spq_prod_idx);
mmiowb();
}
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
index 76a4668..8e0a317 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -170,7 +170,9 @@ static int bnx2x_send_msg2pf(struct bnx2x *bp, u8 *done, dma_addr_t msg_mapping)
wmb();
/* Trigger the PF FW */
- writeb(1, &zone_data->trigger.vf_pf_channel.addr_valid);
+ writeb_relaxed(1, &zone_data->trigger.vf_pf_channel.addr_valid);
+
+ mmiowb();
/* Wait for PF to complete */
while ((tout >= 0) && (!*done)) {
--
2.7.4
^ permalink raw reply related
* [PATCH v7 3/7] bnx2x: Replace doorbell barrier() with wmb()
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Ariel Elior,
everest-linux-l2, linux-kernel
In-Reply-To: <1521988761-30344-1-git-send-email-okaya@codeaurora.org>
barrier() doesn't guarantee memory writes to be observed by the hardware on
all architectures. barrier() only tells compiler not to move this code
with respect to other read/writes.
If memory write needs to be observed by the HW, wmb() is the right choice.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 3 ++-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index d7c98e8..0f86f18 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4153,7 +4153,8 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
wmb();
txdata->tx_db.data.prod += nbd;
- barrier();
+ /* make sure descriptor update is observed by HW */
+ wmb();
DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 1e33abd..39af4f8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -2591,7 +2591,8 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode)
wmb();
txdata->tx_db.data.prod += 2;
- barrier();
+ /* make sure descriptor update is observed by the HW */
+ wmb();
DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
mmiowb();
--
2.7.4
^ permalink raw reply related
* [PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich; +Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya
Code includes wmb() followed by writel() in multiple places. writel()
already has a barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
I did a regex search for wmb() followed by writel() in each drivers
directory.
I scrubbed the ones I care about in this series.
I considered "ease of change", "popular usage" and "performance critical
path" as the determining criteria for my filtering.
We used relaxed API heavily on ARM for a long time but
it did not exist on other architectures. For this reason, relaxed
architectures have been paying double penalty in order to use the common
drivers.
Now that relaxed API is present on all architectures, we can go and scrub
all drivers to see what needs to change and what can remain.
We start with mostly used ones and hope to increase the coverage over time.
It will take a while to cover all drivers.
Feel free to apply patches individually.
Changes since v6:
- bring back amazon ena and add mmiowb, remove
ena_com_write_sq_doorbell_rel().
- remove extra mmiowb in bnx2x
- correct spelling mistake in bnx2x: Replace doorbell barrier() with wmb()
Sinan Kaya (7):
net: qla3xxx: Eliminate duplicate barriers on weakly-ordered archs
qlcnic: Eliminate duplicate barriers on weakly-ordered archs
bnx2x: Replace doorbell barrier() with wmb()
bnx2x: Eliminate duplicate barriers on weakly-ordered archs
net: qlge: Eliminate duplicate barriers on weakly-ordered archs
bnxt_en: Eliminate duplicate barriers on weakly-ordered archs
net: ena: Eliminate duplicate barriers on weakly-ordered archs
drivers/net/ethernet/amazon/ena/ena_com.c | 8 ++++++--
drivers/net/ethernet/amazon/ena/ena_eth_com.h | 8 ++++++--
drivers/net/ethernet/amazon/ena/ena_netdev.c | 5 +++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 12 ++++++++----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 5 +++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 5 +++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 4 +++-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 9 +++++++++
drivers/net/ethernet/qlogic/qla3xxx.c | 5 +++--
drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 2 +-
drivers/net/ethernet/qlogic/qlge/qlge.h | 16 ++++++++++++++++
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 3 ++-
15 files changed, 68 insertions(+), 24 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v7 7/7] net: ena: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Netanel Belgazal,
Saeed Bishara, Zorik Machulsky, David S. Miller, Tobias Klauser,
linux-kernel
In-Reply-To: <1521988761-30344-1-git-send-email-okaya@codeaurora.org>
Code includes barrier() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Create a new wrapper function with relaxed write operator. Use the new
wrapper when a write is following a barrier().
Since code already has an explicit barrier call, changing writel() to
writel_relaxed() and adding mmiowb() for ordering protection.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 8 ++++++--
drivers/net/ethernet/amazon/ena/ena_eth_com.h | 8 ++++++--
drivers/net/ethernet/amazon/ena/ena_netdev.c | 5 +++--
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index bf2de52..1b9d3130 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -631,8 +631,10 @@ static u32 ena_com_reg_bar_read32(struct ena_com_dev *ena_dev, u16 offset)
*/
wmb();
- writel(mmio_read_reg, ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
+ writel_relaxed(mmio_read_reg,
+ ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
+ mmiowb();
for (i = 0; i < timeout; i++) {
if (read_resp->req_id == mmio_read->seq_num)
break;
@@ -1826,7 +1828,9 @@ void ena_com_aenq_intr_handler(struct ena_com_dev *dev, void *data)
/* write the aenq doorbell after all AENQ descriptors were read */
mb();
- writel((u32)aenq->head, dev->reg_bar + ENA_REGS_AENQ_HEAD_DB_OFF);
+ writel_relaxed((u32)aenq->head,
+ dev->reg_bar + ENA_REGS_AENQ_HEAD_DB_OFF);
+ mmiowb();
}
int ena_com_dev_reset(struct ena_com_dev *ena_dev,
diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
index 2f76572..6fdc753 100644
--- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
+++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
@@ -107,7 +107,8 @@ static inline int ena_com_sq_empty_space(struct ena_com_io_sq *io_sq)
return io_sq->q_depth - 1 - cnt;
}
-static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
+static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq,
+ bool relaxed)
{
u16 tail;
@@ -116,7 +117,10 @@ static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
pr_debug("write submission queue doorbell for queue: %d tail: %d\n",
io_sq->qid, tail);
- writel(tail, io_sq->db_addr);
+ if (relaxed)
+ writel_relaxed(tail, io_sq->db_addr);
+ else
+ writel(tail, io_sq->db_addr);
return 0;
}
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 6975150..a822e70 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -556,7 +556,8 @@ static int ena_refill_rx_bufs(struct ena_ring *rx_ring, u32 num)
* issue a doorbell
*/
wmb();
- ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq);
+ ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq, true);
+ mmiowb();
}
rx_ring->next_to_use = next_to_use;
@@ -2151,7 +2152,7 @@ static netdev_tx_t ena_start_xmit(struct sk_buff *skb, struct net_device *dev)
if (netif_xmit_stopped(txq) || !skb->xmit_more) {
/* trigger the dma engine */
- ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
+ ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq, false);
u64_stats_update_begin(&tx_ring->syncp);
tx_ring->tx_stats.doorbells++;
u64_stats_update_end(&tx_ring->syncp);
--
2.7.4
^ permalink raw reply related
* [PATCH v7 6/7] bnxt_en: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Michael Chan,
linux-kernel
In-Reply-To: <1521988761-30344-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a barrier on
some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Create a new wrapper function with relaxed write operator. Use the new
wrapper when a write is following a wmb().
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Also add mmiowb() so that write code doesn't move outside of scope.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1500243..befb538 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1922,7 +1922,7 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
/* Sync BD data before updating doorbell */
wmb();
- bnxt_db_write(bp, db, DB_KEY_TX | prod);
+ bnxt_db_write_relaxed(bp, db, DB_KEY_TX | prod);
}
cpr->cp_raw_cons = raw_cons;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 1989c47..5e453b9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1401,6 +1401,15 @@ static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
((txr->tx_prod - txr->tx_cons) & bp->tx_ring_mask);
}
+/* For TX and RX ring doorbells with no ordering guarantee*/
+static inline void bnxt_db_write_relaxed(struct bnxt *bp, void __iomem *db,
+ u32 val)
+{
+ writel_relaxed(val, db);
+ if (bp->flags & BNXT_FLAG_DOUBLE_DB)
+ writel_relaxed(val, db);
+}
+
/* For TX and RX ring doorbells */
static inline void bnxt_db_write(struct bnxt *bp, void __iomem *db, u32 val)
{
--
2.7.4
^ permalink raw reply related
* [PATCH v7 5/7] net: qlge: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Harish Patil,
Manish Chopra, Dept-GELinuxNICDev, linux-kernel
In-Reply-To: <1521988761-30344-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a barrier on
some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Create a new wrapper function with relaxed write operator. Use the new
wrapper when a write is following a wmb().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/qlogic/qlge/qlge.h | 16 ++++++++++++++++
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 3 ++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index 84ac50f..3e71b65 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -2185,6 +2185,22 @@ static inline void ql_write_db_reg(u32 val, void __iomem *addr)
}
/*
+ * Doorbell Registers:
+ * Doorbell registers are virtual registers in the PCI memory space.
+ * The space is allocated by the chip during PCI initialization. The
+ * device driver finds the doorbell address in BAR 3 in PCI config space.
+ * The registers are used to control outbound and inbound queues. For
+ * example, the producer index for an outbound queue. Each queue uses
+ * 1 4k chunk of memory. The lower half of the space is for outbound
+ * queues. The upper half is for inbound queues.
+ * Caller has to guarantee ordering.
+ */
+static inline void ql_write_db_reg_relaxed(u32 val, void __iomem *addr)
+{
+ writel_relaxed(val, addr);
+}
+
+/*
* Shadow Registers:
* Outbound queues have a consumer index that is maintained by the chip.
* Inbound queues have a producer index that is maintained by the chip.
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 50038d9..8293c202 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -2700,7 +2700,8 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
tx_ring->prod_idx = 0;
wmb();
- ql_write_db_reg(tx_ring->prod_idx, tx_ring->prod_idx_db_reg);
+ ql_write_db_reg_relaxed(tx_ring->prod_idx, tx_ring->prod_idx_db_reg);
+ mmiowb();
netif_printk(qdev, tx_queued, KERN_DEBUG, qdev->ndev,
"tx queued, slot %d, len %d\n",
tx_ring->prod_idx, skb->len);
--
2.7.4
^ permalink raw reply related
* [PATCH v7 2/7] qlcnic: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Harish Patil,
Manish Chopra, Dept-GELinuxNICDev, linux-kernel
In-Reply-To: <1521988761-30344-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing
the register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
Acked-by: Manish Chopra <manish.chopra@cavium.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 46b0372..97c146e7 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -478,7 +478,7 @@ irqreturn_t qlcnic_83xx_clear_legacy_intr(struct qlcnic_adapter *adapter)
wmb();
/* clear the interrupt trigger control register */
- writel(0, adapter->isr_int_vec);
+ writel_relaxed(0, adapter->isr_int_vec);
intr_val = readl(adapter->isr_int_vec);
do {
intr_val = readl(adapter->tgt_status_reg);
--
2.7.4
^ permalink raw reply related
* [PATCH v7 1/7] net: qla3xxx: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-25 14:39 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Dept-GELinuxNICDev,
linux-kernel
In-Reply-To: <1521988761-30344-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing
the register write.
Since code already has an explicit barrier call, changing code to
wmb()
writel_relaxed()
mmiowb()
for multi-arch support.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/qlogic/qla3xxx.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
index 9e5264d..b48f761 100644
--- a/drivers/net/ethernet/qlogic/qla3xxx.c
+++ b/drivers/net/ethernet/qlogic/qla3xxx.c
@@ -1858,8 +1858,9 @@ static void ql_update_small_bufq_prod_index(struct ql3_adapter *qdev)
qdev->small_buf_release_cnt -= 8;
}
wmb();
- writel(qdev->small_buf_q_producer_index,
- &port_regs->CommonRegs.rxSmallQProducerIndex);
+ writel_relaxed(qdev->small_buf_q_producer_index,
+ &port_regs->CommonRegs.rxSmallQProducerIndex);
+ mmiowb();
}
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] nfp: use full 40 bits of the NSP buffer address
From: Jakub Kicinski @ 2018-03-25 14:28 UTC (permalink / raw)
To: davem; +Cc: netdev, oss-drivers, Dirk van der Merwe
In-Reply-To: <20180324024222.20134-1-jakub.kicinski@netronome.com>
On Fri, 23 Mar 2018 19:42:22 -0700, Jakub Kicinski wrote:
> From: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
>
> The NSP default buffer is a piece of NFP memory where additional
> command data can be placed. Its format has been copied from
> host buffer, but the PCIe selection bits do not make sense in
> this case. If those get masked out from a NFP address - writes
> to random place in the chip memory may be issued and crash the
> device.
>
> This has never been an issue because the buffer used to be
> allocated in memory with less-than-38-bit-long address but that
> is about to change.
>
> Signed-off-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
> index 39abac678b71..b54ab02f5b33 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
> @@ -71,8 +71,7 @@
> /* CPP address to retrieve the data from */
> #define NSP_BUFFER 0x10
> #define NSP_BUFFER_CPP GENMASK_ULL(63, 40)
> -#define NSP_BUFFER_PCIE GENMASK_ULL(39, 38)
> -#define NSP_BUFFER_ADDRESS GENMASK_ULL(37, 0)
> +#define NSP_BUFFER_ADDRESS GENMASK_ULL(39, 0)
self-nack, this is changing a shared define for all bufs, adding a new
define of NFP buf will be cleaner.
^ permalink raw reply
* Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink
From: David Ahern @ 2018-03-25 14:27 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, roopa, shm, jiri, idosch, David Ahern
In-Reply-To: <20180324233524.41ecd368@cakuba.netronome.com>
On 3/25/18 12:35 AM, Jakub Kicinski wrote:
> On Sat, 24 Mar 2018 09:02:45 -0600, David Ahern wrote:
>>>> diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
>>>> index 09388c06171d..449b2a1a1800 100644
>>>> --- a/drivers/net/netdevsim/Makefile
>>>> +++ b/drivers/net/netdevsim/Makefile
>>>> @@ -9,3 +9,7 @@ ifeq ($(CONFIG_BPF_SYSCALL),y)
>>>> netdevsim-objs += \
>>>> bpf.o
>>>> endif
>>>> +
>>>> +ifneq ($(CONFIG_NET_DEVLINK),)
>>>
>>> Hm. Don't you need MAY_USE_DEVLINK dependency perhaps?
>>
>> mlxsw uses CONFIG_NET_DEVLINK in its Makefile.
>>
>> MAY_USE_DEVLINK seems to only be used in Kconfig files. Not clear to me
>> why it is needed at all.
>
> NETDEVSIM=y && DEVLINK=m
>
ok. I purposely did not add DEVLINK as a dependency to netdevsim to make
the resource controller truly optional. Can add it if you prefer.
^ permalink raw reply
* Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink
From: David Ahern @ 2018-03-25 14:24 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski,
David Ahern
In-Reply-To: <20180324160238.GE1891@nanopsycho>
On 3/24/18 10:02 AM, Jiri Pirko wrote:
>>>>>>>
>>>>>>> Wait a second. What do you mean by "per-network namespace"? Devlink
>>>>>>> instance is always associated with one physical device. Like an ASIC.
>>>>>>>
>>>>>>>
>>>>>>>> has a net entry, the simplest design is to put it into the namespace of
>>>>>>>> the controller. Without it, controlling resource sizes in namespace
>>>>>>>> 'foobar' has to be done from init_net, which is just wrong.
>>>>>>
>>>>>> you need to look at how netdevsim creates a device per netdevice.
>>>>>
>>>>> That means one devlink instance for each netdevsim device, doesn't it?
>>>>>
>>>>
>>>> yes.
>>>
>>> Still not sure how to handle namespaces in devlink. Originally, I
>>> thought it would be okay to leave all devlink instances in init_ns.
>>> Because what happens if you move netdev to another namespace? Should the
>>> devlink move as well? What if you have multiple ports, each in different
>>> namespace. Can user move devlink instance to another namespace? Etc.
>>>
>>
>> The devlink instance is associated with a 'struct device' and those do
>> not change namespaces AFAIK.
>
> Yeah. But you put devlink instance into namespace according to struct
> net_device. That is mismatch.
>
New netdevsim netdevice creates a new 'struct device' which creates a
new devlink instance. The namespace the netdev is created in is then
passed to the devlink instance. Yes, the netdev could change namespaces,
but that is something we can easily prevent if it has a devlink instance.
But really, we are way down a tangent with respect to the intent of this
patch set. I am fine with limiting the example resource controller to
just the init_net; this patch set is really aimed at the bigger idea of
allowing a notifier handler to fail route and rule adds.
We can revisit devlink and namespaces another time, along with moving
switch ports to namespaces - a key part of implementing a feature
equivalent to what Cisco calls a VDC [1].
[1]
https://www.cisco.com/c/en/us/products/collateral/switches/nexus-7000-10-slot-switch/White_Paper_Tech_Overview_Virtual_Device_Contexts.html
^ permalink raw reply
* Re: [PATCH RFC net-next 1/7] net: Fix fib notifer to return errno
From: David Ahern @ 2018-03-25 14:00 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski
In-Reply-To: <20180325081632.GA11327@splinter>
On 3/25/18 2:16 AM, Ido Schimmel wrote:
> On Thu, Mar 22, 2018 at 03:57:51PM -0700, David Ahern wrote:
>> Notifier handlers use notifier_from_errno to convert any potential error
>> to an encoded format. As a consequence the other side, call_fib_notifiers
>> in this case, needs to use notifier_to_errno to return the error from
>> the handler back to its caller.
>>
>> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
>> ---
>> net/core/fib_notifier.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
>> index 5ace0705a3f9..14ba52ebe8c9 100644
>> --- a/net/core/fib_notifier.c
>> +++ b/net/core/fib_notifier.c
>> @@ -21,8 +21,11 @@ EXPORT_SYMBOL(call_fib_notifier);
>> int call_fib_notifiers(struct net *net, enum fib_event_type event_type,
>> struct fib_notifier_info *info)
>
> There's another (less interesting case) - call_fib_notifier(), which is
> used to dump the FIB tables for the caller registering to the
> notification chain.
>
> For example, if you have a non-default FIB rule in the system and you
> modprobe mlxsw, you'll get a silent failure and routes will not be
> offloaded. On the other hand, I'm not sure we want to fail the module
> loading in such cases.
right. In normal cases the driver is loaded to create the netdevices
long before any networking config is done. So it seems to me the use
case you refer to, some user would have go out of there way to create a
situation where they install config that is not supported by the driver.
>
> A possible solution is to have the driver emit a warning via extack for
> each route/rule being notified after the abort mechanism was triggered.
extack is not available on module load.
Per past discussions, something you suggested, we need a message for
"out-of-line" cases like this where a driver notifies userspace of a
problem.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox