[PATCH] ipvlan: no longer rely on RTNL in ipvlan_nl_fillinfo()
From: Kishore Gummadidala <hidden>
Date: 2026-09-28 21:05:25
Subsystem:
networking drivers, the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
ipvlan_nl_fillinfo() currently relies on RTNL being held because it
calls ipvlan_port_get_rtnl(ipvlan->phy_dev) to retrieve the ipvl_port,
forcing "ip link show" dumps to hold RTNL.
However, ipvlan->port is already initialized in ipvlan_init() with a
reference on port->count and remains valid until ipvlan_uninit(), after
the net_device has been unregistered. Both ipvlan_nl_fillinfo() and
ipvlan_nl_changelink() can therefore use ipvlan->port directly without
dereferencing phy_dev->rx_handler_data or checking for a NULL port.
In addition, port->mode and port->flags are updated under RTNL (via
ipvlan_link_new(), ipvlan_nl_changelink(), and ipvlan_set_port_mode()),
while being read locklessly from the data path and from
ipvlan_nl_fillinfo():
- ipvlan_queue_xmit(), ipvlan_handle_frame(), and ipvlan_skb_to_addr()
read port->mode,
- ipvlan_is_private() and ipvlan_is_vepa() read port->flags.
Furthermore, ipvlan_nl_changelink() previously updated port->flags via
separate read-modify-write calls for IPVLAN_F_PRIVATE and IPVLAN_F_VEPA,
which could publish an intermediate value to concurrent readers. Since
ipvlan_nl_validate() already validates that only those two flags exist
and are mutually exclusive, replace the bit-manipulation helpers with a
single WRITE_ONCE(port->flags, ...) in ipvlan_nl_changelink() (matching
ipvlan_link_new()) and remove the unused ipvlan_{mark,clear}_{private,
vepa}() helpers.
Annotate all remaining accesses to port->mode and port->flags with
READ_ONCE() and WRITE_ONCE(), caching READ_ONCE(port->mode) in a local
variable in ipvlan_queue_xmit() and ipvlan_handle_frame() so the switch
statement and fallback WARN_ONCE() observe a consistent value.
Finally, add const qualifiers to local pointers in ipvlan_nl_fillinfo().
Signed-off-by: Kishore Gummadidala <redacted>
---
drivers/net/ipvlan/ipvlan.h | 24 ++------------------
drivers/net/ipvlan/ipvlan_core.c | 12 ++++++----
drivers/net/ipvlan/ipvlan_l3s.c | 2 +-
drivers/net/ipvlan/ipvlan_main.c | 38 ++++++++------------------------
4 files changed, 20 insertions(+), 56 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 8d05ad480438..48176b546a46 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h@@ -125,32 +125,12 @@ static inline struct ipvl_port *ipvlan_port_get_rtnl(const struct net_device *d) static inline bool ipvlan_is_private(const struct ipvl_port *port) { - return !!(port->flags & IPVLAN_F_PRIVATE); -} - -static inline void ipvlan_mark_private(struct ipvl_port *port) -{ - port->flags |= IPVLAN_F_PRIVATE; -} - -static inline void ipvlan_clear_private(struct ipvl_port *port) -{ - port->flags &= ~IPVLAN_F_PRIVATE; + return !!(READ_ONCE(port->flags) & IPVLAN_F_PRIVATE); } static inline bool ipvlan_is_vepa(const struct ipvl_port *port) { - return !!(port->flags & IPVLAN_F_VEPA); -} - -static inline void ipvlan_mark_vepa(struct ipvl_port *port) -{ - port->flags |= IPVLAN_F_VEPA; -} - -static inline void ipvlan_clear_vepa(struct ipvl_port *port) -{ - port->flags &= ~IPVLAN_F_VEPA; + return !!(READ_ONCE(port->flags) & IPVLAN_F_VEPA); } void ipvlan_init_secret(void);
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 7ad12dc7845c..c08d2bf88503 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c@@ -676,6 +676,7 @@ int ipvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev) { struct ipvl_dev *ipvlan = netdev_priv(dev); struct ipvl_port *port = ipvlan_port_get_rcu_bh(ipvlan->phy_dev); + u16 mode; if (!port) goto out;
@@ -683,7 +684,8 @@ int ipvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev) if (unlikely(!pskb_may_pull(skb, sizeof(struct ethhdr)))) goto out; - switch(port->mode) { + mode = READ_ONCE(port->mode); + switch (mode) { case IPVLAN_MODE_L2: return ipvlan_xmit_mode_l2(skb, dev); case IPVLAN_MODE_L3:
@@ -694,7 +696,7 @@ int ipvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev) } /* Should not reach here */ - WARN_ONCE(true, "%s called for mode = [%x]\n", __func__, port->mode); + WARN_ONCE(true, "%s called for mode = [%x]\n", __func__, mode); out: kfree_skb(skb); return NET_XMIT_DROP;
@@ -782,11 +784,13 @@ rx_handler_result_t ipvlan_handle_frame(struct sk_buff **pskb) { struct sk_buff *skb = *pskb; struct ipvl_port *port = ipvlan_port_get_rcu(skb->dev); + u16 mode; if (!port) return RX_HANDLER_PASS; - switch (port->mode) { + mode = READ_ONCE(port->mode); + switch (mode) { case IPVLAN_MODE_L2: return ipvlan_handle_mode_l2(pskb, port); case IPVLAN_MODE_L3:
@@ -798,7 +802,7 @@ rx_handler_result_t ipvlan_handle_frame(struct sk_buff **pskb) } /* Should not reach here */ - WARN_ONCE(true, "%s called for mode = [%x]\n", __func__, port->mode); + WARN_ONCE(true, "%s called for mode = [%x]\n", __func__, mode); kfree_skb(skb); return RX_HANDLER_CONSUMED; }
diff --git a/drivers/net/ipvlan/ipvlan_l3s.c b/drivers/net/ipvlan/ipvlan_l3s.c
index 7c017fe35522..3e9f5f051d86 100644
--- a/drivers/net/ipvlan/ipvlan_l3s.c
+++ b/drivers/net/ipvlan/ipvlan_l3s.c@@ -24,7 +24,7 @@ static struct ipvl_addr *ipvlan_skb_to_addr(struct sk_buff *skb, goto out; port = ipvlan_port_get_rcu(dev); - if (!port || port->mode != IPVLAN_MODE_L3S) + if (!port || READ_ONCE(port->mode) != IPVLAN_MODE_L3S) goto out; lyr3h = ipvlan_get_L3_hdr(port, skb, &addr_type);
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 4939cf67b336..9c7c1a1f41c3 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c@@ -47,7 +47,7 @@ static int ipvlan_set_port_mode(struct ipvl_port *port, u16 nval, /* Old mode was L3S */ ipvlan_l3s_unregister(port); } - port->mode = nval; + WRITE_ONCE(port->mode, nval); mutex_unlock(&port->pnodes_lock); }
@@ -501,7 +501,7 @@ static int ipvlan_nl_changelink(struct net_device *dev, struct netlink_ext_ack *extack) { struct ipvl_dev *ipvlan = netdev_priv(dev); - struct ipvl_port *port = ipvlan_port_get_rtnl(ipvlan->phy_dev); + struct ipvl_port *port = ipvlan->port; int err = 0; if (!data)
@@ -516,17 +516,7 @@ static int ipvlan_nl_changelink(struct net_device *dev, } if (!err && data[IFLA_IPVLAN_FLAGS]) { - u16 flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]); - - if (flags & IPVLAN_F_PRIVATE) - ipvlan_mark_private(port); - else - ipvlan_clear_private(port); - - if (flags & IPVLAN_F_VEPA) - ipvlan_mark_vepa(port); - else - ipvlan_clear_vepa(port); + WRITE_ONCE(port->flags, nla_get_u16(data[IFLA_IPVLAN_FLAGS])); } return err;
@@ -570,23 +560,13 @@ static int ipvlan_nl_validate(struct nlattr *tb[], struct nlattr *data[], static int ipvlan_nl_fillinfo(struct sk_buff *skb, const struct net_device *dev) { - struct ipvl_dev *ipvlan = netdev_priv(dev); - struct ipvl_port *port = ipvlan_port_get_rtnl(ipvlan->phy_dev); - int ret = -EINVAL; - - if (!port) - goto err; - - ret = -EMSGSIZE; - if (nla_put_u16(skb, IFLA_IPVLAN_MODE, port->mode)) - goto err; - if (nla_put_u16(skb, IFLA_IPVLAN_FLAGS, port->flags)) - goto err; + const struct ipvl_dev *ipvlan = netdev_priv(dev); + const struct ipvl_port *port = ipvlan->port; + if (nla_put_u16(skb, IFLA_IPVLAN_MODE, READ_ONCE(port->mode)) || + nla_put_u16(skb, IFLA_IPVLAN_FLAGS, READ_ONCE(port->flags))) + return -EMSGSIZE; return 0; - -err: - return ret; } int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params,
@@ -682,7 +662,7 @@ int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params, * to be consistent in setting it just like the mode attribute. */ if (data && data[IFLA_IPVLAN_FLAGS]) - port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]); + WRITE_ONCE(port->flags, nla_get_u16(data[IFLA_IPVLAN_FLAGS])); if (data && data[IFLA_IPVLAN_MODE]) mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
--
2.56.0.rc1.315.gc6ed9934b7-goog