| Message ID | 20200928202631.52020-1-kamalheib1@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-rc] RDMA/ipoib: Set rtnl_link_ops for ipoib interfaces | expand |
On Mon, Sep 28, 2020 at 11:26:31PM +0300, Kamal Heib wrote: > Before this patch, the rtnl_link_ops are set only for ipoib network > devices that are created via the rtnl_link_ops->newlink() callback, this > patch fixes that by setting the rtnl_link_ops for all ipoib network > devices. Also, implement the dellink() callback to block users from > trying to remove the base ipoib network device while allowing it only > for child interfaces. Why? Jason
On Mon, Sep 28, 2020 at 07:36:02PM -0300, Jason Gunthorpe wrote: > On Mon, Sep 28, 2020 at 11:26:31PM +0300, Kamal Heib wrote: > > Before this patch, the rtnl_link_ops are set only for ipoib network > > devices that are created via the rtnl_link_ops->newlink() callback, this > > patch fixes that by setting the rtnl_link_ops for all ipoib network > > devices. Also, implement the dellink() callback to block users from > > trying to remove the base ipoib network device while allowing it only > > for child interfaces. > > Why? > This is needed to avoid the inconsistent user experience for PKeys that is created via netlink VS PKeys that is created via sysfs and the based ipoib interface, as you can see below the ipoib attributes are reported only for PKeys that is created via netlink in the 'ip -d link show' output: PKey created via netlink (pkey, mode, and umcast attributes are present): $ ip link add link mlx5_ib0 name mlx5_ib0.8001 type ipoib pkey 0x8001 $ ip -d link show dev mlx5_ib0.8001 28: mlx5_ib0.8001@mlx5_ib0: <BROADCAST,MULTICAST> mtu 4092 qdisc noop state DOWN mode DEFAULT group default qlen 256 link/infiniband 00:00:1f:e3:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a3:19:64 brd 00:ff:ff:ff:ff:12:40:1b:80:01:00:00:00:00:00:00:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65520 ipoib pkey 0x8001 mode datagram umcast 0000 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 While: PKey created via sysfs (the attributes are not present): $ ip -d link show dev mlx5_ib0.8002 20: mlx5_ib0.8002@mlx5_ib0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc mq state UP mode DEFAULT group default qlen 256 link/infiniband 00:00:11:7b:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a3:19:64 brd 00:ff:ff:ff:ff:12:40:1b:80:02:00:00:00:00:00:00:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65520 addrgenmode none numtxqueues 256 numrxqueues 32 gso_max_size 65536 gso_max_segs 65535 Same for the base interface: $ ip -d link show dev mlx5_ib0 19: mlx5_ib0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc mq state UP mode DEFAULT group default qlen 256 link/infiniband 00:00:11:79:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a3:19:64 brd 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65520 addrgenmode none numtxqueues 256 numrxqueues 32 gso_max_size 65536 gso_max_segs 65535 altname ibp7s0f0 After applying this patch: $ ip link add link mlx5_ib0 name mlx5_ib0.8001 type ipoib pkey 0x8001 $ ip -d link show dev mlx5_ib0.8001 38: mlx5_ib0.8001@mlx5_ib0: <BROADCAST,MULTICAST> mtu 4092 qdisc noop state DOWN mode DEFAULT group default qlen 256 link/infiniband 00:00:2e:4e:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a3:19:64 brd 00:ff:ff:ff:ff:12:40:1b:80:01:00:00:00:00:00:00:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65520 ipoib pkey 0x8001 mode datagram umcast 0000 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 $ ip -d link show dev mlx5_ib0.8002 30: mlx5_ib0.8002@mlx5_ib0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc mq state UP mode DEFAULT group default qlen 256 link/infiniband 00:00:1f:e6:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a3:19:64 brd 00:ff:ff:ff:ff:12:40:1b:80:02:00:00:00:00:00:00:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65520 ipoib pkey 0x8002 mode datagram umcast 0000 addrgenmode none numtxqueues 256 numrxqueues 32 gso_max_size 65536 gso_max_segs 65535 $ ip -d link show dev mlx5_ib0 29: mlx5_ib0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc mq state UP mode DEFAULT group default qlen 256 link/infiniband 00:00:1f:e4:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a3:19:64 brd 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65520 ipoib pkey 0xffff mode datagram umcast 0000 addrgenmode none numtxqueues 256 numrxqueues 32 gso_max_size 65536 gso_max_segs 65535 Also modifying the the ipoib attributes will work only for PKeys that is created via netlink (for example, setting the mode). Thanks, Kamal > Jason
On Tue, Sep 29, 2020 at 09:04:38AM +0300, Kamal Heib wrote: > On Mon, Sep 28, 2020 at 07:36:02PM -0300, Jason Gunthorpe wrote: > > On Mon, Sep 28, 2020 at 11:26:31PM +0300, Kamal Heib wrote: > > > Before this patch, the rtnl_link_ops are set only for ipoib network > > > devices that are created via the rtnl_link_ops->newlink() callback, this > > > patch fixes that by setting the rtnl_link_ops for all ipoib network > > > devices. Also, implement the dellink() callback to block users from > > > trying to remove the base ipoib network device while allowing it only > > > for child interfaces. > > > > Why? > > > > This is needed to avoid the inconsistent user experience for PKeys that > is created via netlink VS PKeys that is created via sysfs and the based > ipoib interface, as you can see below the ipoib attributes are reported > only for PKeys that is created via netlink in the 'ip -d link show' > output: Summarize this in the commit message please Jason
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index ab75b7f745d4..96b6be5d507d 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -2477,6 +2477,8 @@ static struct net_device *ipoib_add_port(const char *format, /* call event handler to ensure pkey in sync */ queue_work(ipoib_workqueue, &priv->flush_heavy); + ndev->rtnl_link_ops = ipoib_get_link_ops(); + result = register_netdev(ndev); if (result) { pr_warn("%s: couldn't register ipoib port %d; error %d\n", diff --git a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c index 38c984d16996..d5a90a66b45c 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c @@ -144,6 +144,16 @@ static int ipoib_new_child_link(struct net *src_net, struct net_device *dev, return 0; } +static void ipoib_del_child_link(struct net_device *dev, struct list_head *head) +{ + struct ipoib_dev_priv *priv = ipoib_priv(dev); + + if (!priv->parent) + return; + + unregister_netdevice_queue(dev, head); +} + static size_t ipoib_get_size(const struct net_device *dev) { return nla_total_size(2) + /* IFLA_IPOIB_PKEY */ @@ -158,6 +168,7 @@ static struct rtnl_link_ops ipoib_link_ops __read_mostly = { .priv_size = sizeof(struct ipoib_dev_priv), .setup = ipoib_setup_common, .newlink = ipoib_new_child_link, + .dellink = ipoib_del_child_link, .changelink = ipoib_changelink, .get_size = ipoib_get_size, .fill_info = ipoib_fill_info, diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c index 30865605e098..c60db9f3f5ac 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c @@ -129,6 +129,8 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, goto out_early; } + ndev->rtnl_link_ops = ipoib_get_link_ops(); + result = register_netdevice(ndev); if (result) { ipoib_warn(priv, "failed to initialize; error %i", result);
Before this patch, the rtnl_link_ops are set only for ipoib network devices that are created via the rtnl_link_ops->newlink() callback, this patch fixes that by setting the rtnl_link_ops for all ipoib network devices. Also, implement the dellink() callback to block users from trying to remove the base ipoib network device while allowing it only for child interfaces. Fixes: 9baa0b036410 ("IB/ipoib: Add rtnl_link_ops support") Signed-off-by: Kamal Heib <kamalheib1@gmail.com> --- drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 ++ drivers/infiniband/ulp/ipoib/ipoib_netlink.c | 11 +++++++++++ drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 2 ++ 3 files changed, 15 insertions(+)