From: Kishore Gummadidala <kishorg@google.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Kuniyuki Iwashima <kuniyu@google.com>
Cc: netdev@vger.kernel.org, Kishore Gummadidala <kishorg@google.com>
Subject: [PATCH] ipvlan: no longer rely on RTNL in ipvlan_nl_fillinfo()
Date: Mon, 28 Sep 2026 21:05:08 +0000 [thread overview]
Message-ID: <20260928210508.2645800-1-kishorg@google.com> (raw)
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 <kishorg@google.com>
---
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
next reply other threads:[~2026-09-28 21:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-28 21:05 Kishore Gummadidala [this message]
2026-09-29 5:05 ` [PATCH] ipvlan: no longer rely on RTNL in ipvlan_nl_fillinfo() Eric Dumazet
2026-10-01 12:05 ` netdev-bot+sashiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260928210508.2645800-1-kishorg@google.com \
--to=kishorg@google.com \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=kuniyu@google.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox