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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.