* [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code
@ 2026-01-23 17:03 Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
To: netdev; +Cc: Dmitry Skorodumov
This is a bit stylish patches: The code to handle ipv4 and ipv6
address change are exactly the same. We don't need separate
functions for them. Just look whether we are called
with ipvlan_addr4_notifier_block or with ipvlan_addr6_notifier_block
The changed functionality is already covered
with existing selftests/net/ipvtap_test.sh
v2:
- Fixed warning about unused vars by specifying __maybe_unused
for ipv6-related notifier_blocks
Dmitry Skorodumov (3):
ipvlan: const-specifier for functions that use iaddr
ipvlan: Common code from v6/v4 validator_event
ipvlan: common code to handle ipv6/ipv4 address events
drivers/net/ipvlan/ipvlan.h | 2 +-
drivers/net/ipvlan/ipvlan_core.c | 2 +-
drivers/net/ipvlan/ipvlan_main.c | 188 +++++++++++++------------------
3 files changed, 83 insertions(+), 109 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr
2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
@ 2026-01-23 17:03 ` Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Dmitry Skorodumov, Paolo Abeni,
Kuniyuki Iwashima, Xiao Liang, Ido Schimmel, Julian Vetter,
Guillaume Nault, Eric Dumazet, Stanislav Fomichev, linux-kernel
Cc: Andrew Lunn, David S. Miller
Fix functions that accept "void *iaddr" as param to have
const-specifier.
Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
---
drivers/net/ipvlan/ipvlan.h | 2 +-
drivers/net/ipvlan/ipvlan_core.c | 2 +-
drivers/net/ipvlan/ipvlan_main.c | 6 ++++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 80f84fc87008..235e9218b1bb 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -159,7 +159,7 @@ int ipvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev);
void ipvlan_ht_addr_add(struct ipvl_dev *ipvlan, struct ipvl_addr *addr);
struct ipvl_addr *ipvlan_find_addr(const struct ipvl_dev *ipvlan,
const void *iaddr, bool is_v6);
-bool ipvlan_addr_busy(struct ipvl_port *port, void *iaddr, bool is_v6);
+bool ipvlan_addr_busy(struct ipvl_port *port, const void *iaddr, bool is_v6);
void ipvlan_ht_addr_del(struct ipvl_addr *addr);
struct ipvl_addr *ipvlan_addr_lookup(struct ipvl_port *port, void *lyr3h,
int addr_type, bool use_dest);
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index bdb3a46b327c..6d22487010c0 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -118,7 +118,7 @@ struct ipvl_addr *ipvlan_find_addr(const struct ipvl_dev *ipvlan,
return NULL;
}
-bool ipvlan_addr_busy(struct ipvl_port *port, void *iaddr, bool is_v6)
+bool ipvlan_addr_busy(struct ipvl_port *port, const void *iaddr, bool is_v6)
{
struct ipvl_dev *ipvlan;
bool ret = false;
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index baccdad695fd..cf8c1ea78f4b 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -814,7 +814,8 @@ static int ipvlan_device_event(struct notifier_block *unused,
}
/* the caller must held the addrs lock */
-static int ipvlan_add_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
+static int ipvlan_add_addr(struct ipvl_dev *ipvlan, const void *iaddr,
+ bool is_v6)
{
struct ipvl_addr *addr;
@@ -846,7 +847,8 @@ static int ipvlan_add_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
return 0;
}
-static void ipvlan_del_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
+static void ipvlan_del_addr(struct ipvl_dev *ipvlan, const void *iaddr,
+ bool is_v6)
{
struct ipvl_addr *addr;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
@ 2026-01-23 17:03 ` Dmitry Skorodumov
2026-01-26 18:01 ` Kuniyuki Iwashima
2026-01-23 17:03 ` [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events Dmitry Skorodumov
2026-01-26 15:06 ` [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Simon Horman
3 siblings, 1 reply; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Dmitry Skorodumov, Stanislav Fomichev,
Xiao Liang, Kuniyuki Iwashima, linux-kernel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni
Extract commond code for ipvlan_addr4_validator_event()/
ipvlan_addr6_validator_event() to own function.
Get rid of separate functions for xxx_validator_event()
and check whether we are called for ipv4 or ipv6 by
looking at "notifier_block *nblock" argument
Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
---
v2:
- Fixed warning about unused by specifying __maybe_unused
for ipvlan_addr6_vtor_notifier_block (used only with CONFIG_IPV6)
drivers/net/ipvlan/ipvlan_main.c | 110 +++++++++++++++----------------
1 file changed, 55 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index cf8c1ea78f4b..d5b55876d340 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -878,6 +878,33 @@ static bool ipvlan_is_valid_dev(const struct net_device *dev)
return true;
}
+static int ipvlan_addr_validator_event(struct net_device *dev,
+ unsigned long event,
+ struct netlink_ext_ack *extack,
+ const void *iaddr,
+ bool is_v6)
+{
+ struct ipvl_dev *ipvlan = netdev_priv(dev);
+ int ret = NOTIFY_OK;
+
+ if (!ipvlan_is_valid_dev(dev))
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case NETDEV_UP:
+ spin_lock_bh(&ipvlan->port->addrs_lock);
+ if (ipvlan_addr_busy(ipvlan->port, iaddr, is_v6)) {
+ NL_SET_ERR_MSG(extack,
+ "Address already assigned to an ipvlan device");
+ ret = notifier_from_errno(-EADDRINUSE);
+ }
+ spin_unlock_bh(&ipvlan->port->addrs_lock);
+ break;
+ }
+
+ return ret;
+}
+
#if IS_ENABLED(CONFIG_IPV6)
static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
{
@@ -922,32 +949,6 @@ static int ipvlan_addr6_event(struct notifier_block *unused,
return NOTIFY_OK;
}
-
-static int ipvlan_addr6_validator_event(struct notifier_block *unused,
- unsigned long event, void *ptr)
-{
- struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
- struct net_device *dev = (struct net_device *)i6vi->i6vi_dev->dev;
- struct ipvl_dev *ipvlan = netdev_priv(dev);
- int ret = NOTIFY_OK;
-
- if (!ipvlan_is_valid_dev(dev))
- return NOTIFY_DONE;
-
- switch (event) {
- case NETDEV_UP:
- spin_lock_bh(&ipvlan->port->addrs_lock);
- if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
- NL_SET_ERR_MSG(i6vi->extack,
- "Address already assigned to an ipvlan device");
- ret = notifier_from_errno(-EADDRINUSE);
- }
- spin_unlock_bh(&ipvlan->port->addrs_lock);
- break;
- }
-
- return ret;
-}
#endif
static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
@@ -997,38 +998,15 @@ static int ipvlan_addr4_event(struct notifier_block *unused,
return NOTIFY_OK;
}
-static int ipvlan_addr4_validator_event(struct notifier_block *unused,
- unsigned long event, void *ptr)
-{
- struct in_validator_info *ivi = (struct in_validator_info *)ptr;
- struct net_device *dev = (struct net_device *)ivi->ivi_dev->dev;
- struct ipvl_dev *ipvlan = netdev_priv(dev);
- int ret = NOTIFY_OK;
-
- if (!ipvlan_is_valid_dev(dev))
- return NOTIFY_DONE;
-
- switch (event) {
- case NETDEV_UP:
- spin_lock_bh(&ipvlan->port->addrs_lock);
- if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
- NL_SET_ERR_MSG(ivi->extack,
- "Address already assigned to an ipvlan device");
- ret = notifier_from_errno(-EADDRINUSE);
- }
- spin_unlock_bh(&ipvlan->port->addrs_lock);
- break;
- }
-
- return ret;
-}
+static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
+ unsigned long event, void *ptr);
static struct notifier_block ipvlan_addr4_notifier_block __read_mostly = {
.notifier_call = ipvlan_addr4_event,
};
static struct notifier_block ipvlan_addr4_vtor_notifier_block __read_mostly = {
- .notifier_call = ipvlan_addr4_validator_event,
+ .notifier_call = ipvlan_addr_validator_event_cb,
};
static struct notifier_block ipvlan_notifier_block __read_mostly = {
@@ -1039,11 +1017,33 @@ static struct notifier_block ipvlan_notifier_block __read_mostly = {
static struct notifier_block ipvlan_addr6_notifier_block __read_mostly = {
.notifier_call = ipvlan_addr6_event,
};
+#endif
-static struct notifier_block ipvlan_addr6_vtor_notifier_block __read_mostly = {
- .notifier_call = ipvlan_addr6_validator_event,
+static struct notifier_block
+ipvlan_addr6_vtor_notifier_block __read_mostly __maybe_unused = {
+ .notifier_call = ipvlan_addr_validator_event_cb,
};
-#endif
+
+static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
+ unsigned long event, void *ptr)
+{
+ struct in6_validator_info *i6vi;
+ struct net_device *dev;
+
+ if (nblock == &ipvlan_addr4_vtor_notifier_block) {
+ struct in_validator_info *ivi;
+
+ ivi = (struct in_validator_info *)ptr;
+ dev = ivi->ivi_dev->dev;
+ return ipvlan_addr_validator_event(dev, event, ivi->extack,
+ &ivi->ivi_addr, false);
+ }
+
+ i6vi = (struct in6_validator_info *)ptr;
+ dev = i6vi->i6vi_dev->dev;
+ return ipvlan_addr_validator_event(dev, event, i6vi->extack,
+ &i6vi->i6vi_addr, true);
+}
static int __init ipvlan_init_module(void)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events
2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
@ 2026-01-23 17:03 ` Dmitry Skorodumov
2026-01-26 15:06 ` [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Simon Horman
3 siblings, 0 replies; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Dmitry Skorodumov, Kuniyuki Iwashima,
Stanislav Fomichev, Xiao Liang, linux-kernel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni
Both IPv4 and IPv6 addr-event functions are very similar. Refactor
to use common funcitons.
Get rid of separate functions for ipvlan_addrX_event()
and check whether we are called for ipv4 or ipv6 by
looking at "notifier_block *nblock" argument
Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
---
v2:
- Fixed warning about unused by specifying __maybe_unused
for ipvlan_addr6_notifier_block (used only with CONFIG_IPV6)
drivers/net/ipvlan/ipvlan_main.c | 114 ++++++++++++-------------------
1 file changed, 43 insertions(+), 71 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index d5b55876d340..03d0f23dbe33 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -905,93 +905,45 @@ static int ipvlan_addr_validator_event(struct net_device *dev,
return ret;
}
-#if IS_ENABLED(CONFIG_IPV6)
-static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
+static int ipvlan_add_addr_event(struct ipvl_dev *ipvlan, const void *iaddr,
+ bool is_v6)
{
int ret = -EINVAL;
spin_lock_bh(&ipvlan->port->addrs_lock);
- if (ipvlan_addr_busy(ipvlan->port, ip6_addr, true))
- netif_err(ipvlan, ifup, ipvlan->dev,
- "Failed to add IPv6=%pI6c addr for %s intf\n",
- ip6_addr, ipvlan->dev->name);
- else
- ret = ipvlan_add_addr(ipvlan, ip6_addr, true);
- spin_unlock_bh(&ipvlan->port->addrs_lock);
- return ret;
-}
-
-static void ipvlan_del_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
-{
- return ipvlan_del_addr(ipvlan, ip6_addr, true);
-}
-
-static int ipvlan_addr6_event(struct notifier_block *unused,
- unsigned long event, void *ptr)
-{
- struct inet6_ifaddr *if6 = (struct inet6_ifaddr *)ptr;
- struct net_device *dev = (struct net_device *)if6->idev->dev;
- struct ipvl_dev *ipvlan = netdev_priv(dev);
-
- if (!ipvlan_is_valid_dev(dev))
- return NOTIFY_DONE;
-
- switch (event) {
- case NETDEV_UP:
- if (ipvlan_add_addr6(ipvlan, &if6->addr))
- return NOTIFY_BAD;
- break;
-
- case NETDEV_DOWN:
- ipvlan_del_addr6(ipvlan, &if6->addr);
- break;
+ if (ipvlan_addr_busy(ipvlan->port, iaddr, is_v6)) {
+ if (is_v6) {
+ netif_err(ipvlan, ifup, ipvlan->dev,
+ "Failed to add IPv6=%pI6c addr on %s intf\n",
+ iaddr, ipvlan->dev->name);
+ } else {
+ netif_err(ipvlan, ifup, ipvlan->dev,
+ "Failed to add IPv4=%pI4 on %s intf.\n",
+ iaddr, ipvlan->dev->name);
+ }
+ } else {
+ ret = ipvlan_add_addr(ipvlan, iaddr, is_v6);
}
-
- return NOTIFY_OK;
-}
-#endif
-
-static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
-{
- int ret = -EINVAL;
-
- spin_lock_bh(&ipvlan->port->addrs_lock);
- if (ipvlan_addr_busy(ipvlan->port, ip4_addr, false))
- netif_err(ipvlan, ifup, ipvlan->dev,
- "Failed to add IPv4=%pI4 on %s intf.\n",
- ip4_addr, ipvlan->dev->name);
- else
- ret = ipvlan_add_addr(ipvlan, ip4_addr, false);
spin_unlock_bh(&ipvlan->port->addrs_lock);
return ret;
}
-static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
-{
- return ipvlan_del_addr(ipvlan, ip4_addr, false);
-}
-
-static int ipvlan_addr4_event(struct notifier_block *unused,
- unsigned long event, void *ptr)
+static int ipvlan_addr_event(struct net_device *dev, unsigned long event,
+ const void *iaddr, bool is_v6)
{
- struct in_ifaddr *if4 = (struct in_ifaddr *)ptr;
- struct net_device *dev = (struct net_device *)if4->ifa_dev->dev;
struct ipvl_dev *ipvlan = netdev_priv(dev);
- struct in_addr ip4_addr;
if (!ipvlan_is_valid_dev(dev))
return NOTIFY_DONE;
switch (event) {
case NETDEV_UP:
- ip4_addr.s_addr = if4->ifa_address;
- if (ipvlan_add_addr4(ipvlan, &ip4_addr))
+ if (ipvlan_add_addr_event(ipvlan, iaddr, is_v6))
return NOTIFY_BAD;
break;
case NETDEV_DOWN:
- ip4_addr.s_addr = if4->ifa_address;
- ipvlan_del_addr4(ipvlan, &ip4_addr);
+ ipvlan_del_addr(ipvlan, iaddr, is_v6);
break;
}
@@ -1001,8 +953,11 @@ static int ipvlan_addr4_event(struct notifier_block *unused,
static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
unsigned long event, void *ptr);
+static int ipvlan_addr_event_cb(struct notifier_block *unused,
+ unsigned long event, void *ptr);
+
static struct notifier_block ipvlan_addr4_notifier_block __read_mostly = {
- .notifier_call = ipvlan_addr4_event,
+ .notifier_call = ipvlan_addr_event_cb,
};
static struct notifier_block ipvlan_addr4_vtor_notifier_block __read_mostly = {
@@ -1013,11 +968,10 @@ static struct notifier_block ipvlan_notifier_block __read_mostly = {
.notifier_call = ipvlan_device_event,
};
-#if IS_ENABLED(CONFIG_IPV6)
-static struct notifier_block ipvlan_addr6_notifier_block __read_mostly = {
- .notifier_call = ipvlan_addr6_event,
+static struct notifier_block
+ipvlan_addr6_notifier_block __read_mostly __maybe_unused = {
+ .notifier_call = ipvlan_addr_event_cb,
};
-#endif
static struct notifier_block
ipvlan_addr6_vtor_notifier_block __read_mostly __maybe_unused = {
@@ -1045,6 +999,24 @@ static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
&i6vi->i6vi_addr, true);
}
+static int ipvlan_addr_event_cb(struct notifier_block *nblock,
+ unsigned long event, void *ptr)
+{
+ struct inet6_ifaddr *if6;
+ struct net_device *dev;
+
+ if (nblock == &ipvlan_addr4_notifier_block) {
+ struct in_ifaddr *if4 = (struct in_ifaddr *)ptr;
+
+ dev = if4->ifa_dev->dev;
+ return ipvlan_addr_event(dev, event, &if4->ifa_address, false);
+ }
+
+ if6 = (struct inet6_ifaddr *)ptr;
+ dev = if6->idev->dev;
+ return ipvlan_addr_event(dev, event, &if6->addr, true);
+}
+
static int __init ipvlan_init_module(void)
{
int err;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code
2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
` (2 preceding siblings ...)
2026-01-23 17:03 ` [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events Dmitry Skorodumov
@ 2026-01-26 15:06 ` Simon Horman
3 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2026-01-26 15:06 UTC (permalink / raw)
To: Dmitry Skorodumov; +Cc: netdev, Dmitry Skorodumov
On Fri, Jan 23, 2026 at 08:03:53PM +0300, Dmitry Skorodumov wrote:
> This is a bit stylish patches: The code to handle ipv4 and ipv6
> address change are exactly the same. We don't need separate
> functions for them. Just look whether we are called
> with ipvlan_addr4_notifier_block or with ipvlan_addr6_notifier_block
>
> The changed functionality is already covered
> with existing selftests/net/ipvtap_test.sh
Hi Dmitry,
I'm somewhat ambivalent towards the trade-off between
reducing code duplication, and requiring run-time demuxing
between ipv4 and ipv4 code paths and the moderate increase in
code complexity that entails.
But assuming these paths are not performance sensitive I guess
that reducing duplication has the upper hand.
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
@ 2026-01-26 18:01 ` Kuniyuki Iwashima
2026-01-27 5:56 ` Dmitry Skorodumov
0 siblings, 1 reply; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-01-26 18:01 UTC (permalink / raw)
To: Dmitry Skorodumov
Cc: netdev, Jakub Kicinski, Dmitry Skorodumov, Stanislav Fomichev,
Xiao Liang, linux-kernel, Andrew Lunn, David S. Miller,
Eric Dumazet, Paolo Abeni
On Fri, Jan 23, 2026 at 9:04 AM Dmitry Skorodumov <dskr99@gmail.com> wrote:
>
> Extract commond code for ipvlan_addr4_validator_event()/
> ipvlan_addr6_validator_event() to own function.
>
> Get rid of separate functions for xxx_validator_event()
> and check whether we are called for ipv4 or ipv6 by
> looking at "notifier_block *nblock" argument
>
> Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
> ---
> v2:
> - Fixed warning about unused by specifying __maybe_unused
> for ipvlan_addr6_vtor_notifier_block (used only with CONFIG_IPV6)
>
> drivers/net/ipvlan/ipvlan_main.c | 110 +++++++++++++++----------------
> 1 file changed, 55 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index cf8c1ea78f4b..d5b55876d340 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -878,6 +878,33 @@ static bool ipvlan_is_valid_dev(const struct net_device *dev)
> return true;
> }
>
> +static int ipvlan_addr_validator_event(struct net_device *dev,
> + unsigned long event,
> + struct netlink_ext_ack *extack,
> + const void *iaddr,
> + bool is_v6)
> +{
> + struct ipvl_dev *ipvlan = netdev_priv(dev);
> + int ret = NOTIFY_OK;
> +
> + if (!ipvlan_is_valid_dev(dev))
> + return NOTIFY_DONE;
> +
> + switch (event) {
> + case NETDEV_UP:
> + spin_lock_bh(&ipvlan->port->addrs_lock);
> + if (ipvlan_addr_busy(ipvlan->port, iaddr, is_v6)) {
> + NL_SET_ERR_MSG(extack,
> + "Address already assigned to an ipvlan device");
> + ret = notifier_from_errno(-EADDRINUSE);
> + }
> + spin_unlock_bh(&ipvlan->port->addrs_lock);
> + break;
> + }
> +
> + return ret;
> +}
> +
> #if IS_ENABLED(CONFIG_IPV6)
> static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
> {
> @@ -922,32 +949,6 @@ static int ipvlan_addr6_event(struct notifier_block *unused,
>
> return NOTIFY_OK;
> }
> -
> -static int ipvlan_addr6_validator_event(struct notifier_block *unused,
> - unsigned long event, void *ptr)
> -{
> - struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
> - struct net_device *dev = (struct net_device *)i6vi->i6vi_dev->dev;
> - struct ipvl_dev *ipvlan = netdev_priv(dev);
> - int ret = NOTIFY_OK;
> -
> - if (!ipvlan_is_valid_dev(dev))
> - return NOTIFY_DONE;
> -
> - switch (event) {
> - case NETDEV_UP:
> - spin_lock_bh(&ipvlan->port->addrs_lock);
> - if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
> - NL_SET_ERR_MSG(i6vi->extack,
> - "Address already assigned to an ipvlan device");
> - ret = notifier_from_errno(-EADDRINUSE);
> - }
> - spin_unlock_bh(&ipvlan->port->addrs_lock);
> - break;
> - }
> -
> - return ret;
> -}
> #endif
>
> static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
> @@ -997,38 +998,15 @@ static int ipvlan_addr4_event(struct notifier_block *unused,
> return NOTIFY_OK;
> }
>
> -static int ipvlan_addr4_validator_event(struct notifier_block *unused,
> - unsigned long event, void *ptr)
> -{
> - struct in_validator_info *ivi = (struct in_validator_info *)ptr;
> - struct net_device *dev = (struct net_device *)ivi->ivi_dev->dev;
> - struct ipvl_dev *ipvlan = netdev_priv(dev);
> - int ret = NOTIFY_OK;
> -
> - if (!ipvlan_is_valid_dev(dev))
> - return NOTIFY_DONE;
> -
> - switch (event) {
> - case NETDEV_UP:
> - spin_lock_bh(&ipvlan->port->addrs_lock);
> - if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
> - NL_SET_ERR_MSG(ivi->extack,
> - "Address already assigned to an ipvlan device");
> - ret = notifier_from_errno(-EADDRINUSE);
> - }
> - spin_unlock_bh(&ipvlan->port->addrs_lock);
> - break;
> - }
> -
> - return ret;
> -}
> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
> + unsigned long event, void *ptr);
>
> static struct notifier_block ipvlan_addr4_notifier_block __read_mostly = {
> .notifier_call = ipvlan_addr4_event,
> };
>
> static struct notifier_block ipvlan_addr4_vtor_notifier_block __read_mostly = {
> - .notifier_call = ipvlan_addr4_validator_event,
> + .notifier_call = ipvlan_addr_validator_event_cb,
> };
>
> static struct notifier_block ipvlan_notifier_block __read_mostly = {
> @@ -1039,11 +1017,33 @@ static struct notifier_block ipvlan_notifier_block __read_mostly = {
> static struct notifier_block ipvlan_addr6_notifier_block __read_mostly = {
> .notifier_call = ipvlan_addr6_event,
> };
> +#endif
>
> -static struct notifier_block ipvlan_addr6_vtor_notifier_block __read_mostly = {
> - .notifier_call = ipvlan_addr6_validator_event,
> +static struct notifier_block
> +ipvlan_addr6_vtor_notifier_block __read_mostly __maybe_unused = {
I think #if IS_ENABLED(CONFIG_IPV6) is preferable to
__mabe_unused.
> + .notifier_call = ipvlan_addr_validator_event_cb,
> };
> -#endif
> +
> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
> + unsigned long event, void *ptr)
> +{
> + struct in6_validator_info *i6vi;
> + struct net_device *dev;
> +
> + if (nblock == &ipvlan_addr4_vtor_notifier_block) {
If you check this against the v6 one instead, the if block
and the i6vi definition can be guarded with
#if IS_ENABLED(CONFIG_IPV6)
> + struct in_validator_info *ivi;
> +
> + ivi = (struct in_validator_info *)ptr;
> + dev = ivi->ivi_dev->dev;
> + return ipvlan_addr_validator_event(dev, event, ivi->extack,
> + &ivi->ivi_addr, false);
> + }
> +
> + i6vi = (struct in6_validator_info *)ptr;
> + dev = i6vi->i6vi_dev->dev;
> + return ipvlan_addr_validator_event(dev, event, i6vi->extack,
> + &i6vi->i6vi_addr, true);
> +}
>
> static int __init ipvlan_init_module(void)
> {
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
2026-01-26 18:01 ` Kuniyuki Iwashima
@ 2026-01-27 5:56 ` Dmitry Skorodumov
2026-01-27 7:37 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-27 5:56 UTC (permalink / raw)
To: Kuniyuki Iwashima, Dmitry Skorodumov
Cc: netdev, Jakub Kicinski, Stanislav Fomichev, Xiao Liang,
linux-kernel, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni
On 1/26/2026 9:01 PM, Kuniyuki Iwashima wrote:
> On Fri, Jan 23, 2026 at 9:04 AM Dmitry Skorodumov <dskr99@gmail.com> wrote:
>> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
>> + unsigned long event, void *ptr)
>> +{
>> + struct in6_validator_info *i6vi;
>> + struct net_device *dev;
>> +
>> + if (nblock == &ipvlan_addr4_vtor_notifier_block) {
>
> If you check this against the v6 one instead, the if block
> and the i6vi definition can be guarded with
> #if IS_ENABLED(CONFIG_IPV6)
Hm... Few months ago, I had a conversation here that convinced me: avoid
#ifdef whenever possible. They overburden code and reduce readability.
It even might be a good idea to replace wherever possible preprocessor
checks with runtime checks. Use
if (IS_ENABLED(CONFIG_IPV6)) { ... }
instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif
--
I'm ok with both approaches (though i tend to like runtime checks) - but
unsure what is a common practice in bpf-net
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
2026-01-27 5:56 ` Dmitry Skorodumov
@ 2026-01-27 7:37 ` Eric Dumazet
2026-01-27 7:54 ` Dmitry Skorodumov
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2026-01-27 7:37 UTC (permalink / raw)
To: Dmitry Skorodumov
Cc: Kuniyuki Iwashima, Dmitry Skorodumov, netdev, Jakub Kicinski,
Stanislav Fomichev, Xiao Liang, linux-kernel, Andrew Lunn,
David S. Miller, Paolo Abeni
On Tue, Jan 27, 2026 at 6:56 AM Dmitry Skorodumov
<skorodumov.dmitry@huawei.com> wrote:
>
> On 1/26/2026 9:01 PM, Kuniyuki Iwashima wrote:
> > On Fri, Jan 23, 2026 at 9:04 AM Dmitry Skorodumov <dskr99@gmail.com> wrote:
>
> >> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
> >> + unsigned long event, void *ptr)
> >> +{
> >> + struct in6_validator_info *i6vi;
> >> + struct net_device *dev;
> >> +
> >> + if (nblock == &ipvlan_addr4_vtor_notifier_block) {
> >
> > If you check this against the v6 one instead, the if block
> > and the i6vi definition can be guarded with
> > #if IS_ENABLED(CONFIG_IPV6)
>
> Hm... Few months ago, I had a conversation here that convinced me: avoid
> #ifdef whenever possible. They overburden code and reduce readability.
>
> It even might be a good idea to replace wherever possible preprocessor
> checks with runtime checks. Use
> if (IS_ENABLED(CONFIG_IPV6)) { ... }
>
> instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif
>
> --
>
> I'm ok with both approaches (though i tend to like runtime checks) - but
> unsure what is a common practice in bpf-net
What you call runtime checks is in reality the same : C compiler is
able to optimize constant boolean expressions
if (0) {
code_0;
}
if (0 && anything) {
code_0_1;
}
-> Compiler will completely remove all of this.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
2026-01-27 7:37 ` Eric Dumazet
@ 2026-01-27 7:54 ` Dmitry Skorodumov
2026-01-27 8:11 ` Kuniyuki Iwashima
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-27 7:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: Kuniyuki Iwashima, Dmitry Skorodumov, netdev, Jakub Kicinski,
Stanislav Fomichev, Xiao Liang, linux-kernel, Andrew Lunn,
David S. Miller, Paolo Abeni
On 1/27/2026 10:37 AM, Eric Dumazet wrote:
> On Tue, Jan 27, 2026 at 6:56 AM Dmitry Skorodumov
> <skorodumov.dmitry@huawei.com> wrote:
>> It even might be a good idea to replace wherever possible preprocessor
>> checks with runtime checks. Use
>> if (IS_ENABLED(CONFIG_IPV6)) { ... }
>>
>> instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif
>>
>> --
>>
>> I'm ok with both approaches (though i tend to like runtime checks) - but
>> unsure what is a common practice in bpf-net
>
> What you call runtime checks is in reality the same : C compiler is
> able to optimize constant boolean expressions
Should I fix patches to be something like
if (nblock == &ipvlan_addr4_vtor_notifier_block) {
} else if (IS_ENABLED(CONFIG_IPV6)) {
...
}
but still with __maybe_unused for ipvlan_addr6_vtor_notifier_block ?
I see that this __maybe_unused is used quite often in linux-kernel in
similar way
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
2026-01-27 7:54 ` Dmitry Skorodumov
@ 2026-01-27 8:11 ` Kuniyuki Iwashima
0 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-01-27 8:11 UTC (permalink / raw)
To: Dmitry Skorodumov
Cc: Eric Dumazet, Dmitry Skorodumov, netdev, Jakub Kicinski,
Stanislav Fomichev, Xiao Liang, linux-kernel, Andrew Lunn,
David S. Miller, Paolo Abeni
On Mon, Jan 26, 2026 at 11:54 PM Dmitry Skorodumov
<skorodumov.dmitry@huawei.com> wrote:
>
> On 1/27/2026 10:37 AM, Eric Dumazet wrote:
> > On Tue, Jan 27, 2026 at 6:56 AM Dmitry Skorodumov
> > <skorodumov.dmitry@huawei.com> wrote:
> >> It even might be a good idea to replace wherever possible preprocessor
> >> checks with runtime checks. Use
> >> if (IS_ENABLED(CONFIG_IPV6)) { ... }
> >>
> >> instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif
> >>
> >> --
> >>
> >> I'm ok with both approaches (though i tend to like runtime checks) - but
> >> unsure what is a common practice in bpf-net
> >
> > What you call runtime checks is in reality the same : C compiler is
> > able to optimize constant boolean expressions
>
> Should I fix patches to be something like
>
> if (nblock == &ipvlan_addr4_vtor_notifier_block) {
> } else if (IS_ENABLED(CONFIG_IPV6)) {
> ...
> }
>
> but still with __maybe_unused for ipvlan_addr6_vtor_notifier_block ?
If IS_ENABLED() or #if is used properly, __maybe_unused
should be unnecessary. Actually the patch moved
ipvlan_addr6_vtor_notifier_block out of the existing #if guard.
Also, ipvlan_main.c already has such #if guards.
>
> I see that this __maybe_unused is used quite often in linux-kernel in
> similar way
but not that under net/.
$ grep -rnI __maybe_unused | wc -l
4827
$ grep -rnI __maybe_unused net | wc -l
33
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-01-27 8:11 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
2026-01-26 18:01 ` Kuniyuki Iwashima
2026-01-27 5:56 ` Dmitry Skorodumov
2026-01-27 7:37 ` Eric Dumazet
2026-01-27 7:54 ` Dmitry Skorodumov
2026-01-27 8:11 ` Kuniyuki Iwashima
2026-01-23 17:03 ` [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events Dmitry Skorodumov
2026-01-26 15:06 ` [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Simon Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox