diff mbox series

[iproute2-next,v1,1/1,v1] ip-link: add support for nolocalbypass in vxlan

Message ID 20230323060451.24280-1-vladimir@nikishkin.pw (mailing list archive)
State Superseded
Delegated to: David Ahern
Headers show
Series [iproute2-next,v1,1/1,v1] ip-link: add support for nolocalbypass in vxlan | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Vladimir Nikishkin March 23, 2023, 6:04 a.m. UTC
Add userspace support for the nolocalbypass vxlan netlink
attribute. With nolocalbypass, if an entry is pointing to the
local machine, but the system driver is not listening on this
port, the driver will not drop packets, but will forward them
to the userspace network stack instead.

This commit has a corresponding patch in the net-next list.

Signed-off-by: Vladimir Nikishkin <vladimir@nikishkin.pw>
---
 include/uapi/linux/if_link.h |  1 +
 ip/iplink_vxlan.c            | 18 ++++++++++++++++++
 man/man8/ip-link.8.in        |  8 ++++++++
 3 files changed, 27 insertions(+)

Comments

Hangbin Liu March 24, 2023, 1:38 a.m. UTC | #1
Hi Vladimir,

For the subject prefix, [PATCH iproute2-next] is enough for the v1 patch.

On Thu, Mar 23, 2023 at 02:04:51PM +0800, Vladimir Nikishkin wrote:
> Add userspace support for the nolocalbypass vxlan netlink
> attribute. With nolocalbypass, if an entry is pointing to the
> local machine, but the system driver is not listening on this
> port, the driver will not drop packets, but will forward them
> to the userspace network stack instead.
> 
> This commit has a corresponding patch in the net-next list.
> 
> Signed-off-by: Vladimir Nikishkin <vladimir@nikishkin.pw>
> ---
>  include/uapi/linux/if_link.h |  1 +
>  ip/iplink_vxlan.c            | 18 ++++++++++++++++++
>  man/man8/ip-link.8.in        |  8 ++++++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index d61bd32d..fd390b40 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -824,6 +824,7 @@ enum {
>  	IFLA_VXLAN_TTL_INHERIT,
>  	IFLA_VXLAN_DF,
>  	IFLA_VXLAN_VNIFILTER, /* only applicable with COLLECT_METADATA mode */
> +	IFLA_VXLAN_LOCALBYPASS,
>  	__IFLA_VXLAN_MAX
>  };
>  #define IFLA_VXLAN_MAX	(__IFLA_VXLAN_MAX - 1)

There is no need to include the uapi header. Stephen will sync it with upstream.

Hi Stephen, should we add this note to the README.devel?

> diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> index c7e0e1c4..17fa5cf7 100644
> --- a/ip/iplink_vxlan.c
> +++ b/ip/iplink_vxlan.c
> @@ -276,6 +276,12 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
>  		} else if (!matches(*argv, "noudpcsum")) {
>  			check_duparg(&attrs, IFLA_VXLAN_UDP_CSUM, *argv, *argv);
>  			addattr8(n, 1024, IFLA_VXLAN_UDP_CSUM, 0);
> +		} else if (!matches(*argv, "localbypass")) {
> +			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
> +			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 1);
> +		} else if (!matches(*argv, "nolocalbypass")) {
> +			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
> +			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 0);

matches is deparated, please use strcmp instead.

Thanks
Hangbin
Vladimir Nikishkin April 5, 2023, 5:21 a.m. UTC | #2
Hangbin Liu <liuhangbin@gmail.com> writes:

> Hi Vladimir,
>
> For the subject prefix, [PATCH iproute2-next] is enough for the v1 patch.
>
> On Thu, Mar 23, 2023 at 02:04:51PM +0800, Vladimir Nikishkin wrote:
>> Add userspace support for the nolocalbypass vxlan netlink
>> attribute. With nolocalbypass, if an entry is pointing to the
>> local machine, but the system driver is not listening on this
>> port, the driver will not drop packets, but will forward them
>> to the userspace network stack instead.
>> 
>> This commit has a corresponding patch in the net-next list.
>> 
>> Signed-off-by: Vladimir Nikishkin <vladimir@nikishkin.pw>
>> ---
>>  include/uapi/linux/if_link.h |  1 +
>>  ip/iplink_vxlan.c            | 18 ++++++++++++++++++
>>  man/man8/ip-link.8.in        |  8 ++++++++
>>  3 files changed, 27 insertions(+)
>> 
>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> index d61bd32d..fd390b40 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -824,6 +824,7 @@ enum {
>>  	IFLA_VXLAN_TTL_INHERIT,
>>  	IFLA_VXLAN_DF,
>>  	IFLA_VXLAN_VNIFILTER, /* only applicable with COLLECT_METADATA mode */
>> +	IFLA_VXLAN_LOCALBYPASS,
>>  	__IFLA_VXLAN_MAX
>>  };
>>  #define IFLA_VXLAN_MAX	(__IFLA_VXLAN_MAX - 1)
>
> There is no need to include the uapi header. Stephen will sync it with upstream.
>
> Hi Stephen, should we add this note to the README.devel?
>

Without this change, my code does not compile. I ended up modifying the
header, but not adding it to git. Is this the correct way of doing it?

>> diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
>> index c7e0e1c4..17fa5cf7 100644
>> --- a/ip/iplink_vxlan.c
>> +++ b/ip/iplink_vxlan.c
>> @@ -276,6 +276,12 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
>>  		} else if (!matches(*argv, "noudpcsum")) {
>>  			check_duparg(&attrs, IFLA_VXLAN_UDP_CSUM, *argv, *argv);
>>  			addattr8(n, 1024, IFLA_VXLAN_UDP_CSUM, 0);
>> +		} else if (!matches(*argv, "localbypass")) {
>> +			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
>> +			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 1);
>> +		} else if (!matches(*argv, "nolocalbypass")) {
>> +			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
>> +			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 0);
>
> matches is deparated, please use strcmp instead.

Why is strcmp recommended, not strncmp? I remember strcmp being frowned
upon for some potential memory bounds violations.

>
> Thanks
> Hangbin
Hangbin Liu April 5, 2023, 9:15 a.m. UTC | #3
On Wed, Apr 05, 2023 at 01:21:17PM +0800, Vladimir Nikishkin wrote:
> > There is no need to include the uapi header. Stephen will sync it with upstream.
> >
> > Hi Stephen, should we add this note to the README.devel?
> >
> 
> Without this change, my code does not compile. I ended up modifying the
> header, but not adding it to git. Is this the correct way of doing it?

Yes, that's what I did.

> 
> >> diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> >> index c7e0e1c4..17fa5cf7 100644
> >> --- a/ip/iplink_vxlan.c
> >> +++ b/ip/iplink_vxlan.c
> >> @@ -276,6 +276,12 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
> >>  		} else if (!matches(*argv, "noudpcsum")) {
> >>  			check_duparg(&attrs, IFLA_VXLAN_UDP_CSUM, *argv, *argv);
> >>  			addattr8(n, 1024, IFLA_VXLAN_UDP_CSUM, 0);
> >> +		} else if (!matches(*argv, "localbypass")) {
> >> +			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
> >> +			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 1);
> >> +		} else if (!matches(*argv, "nolocalbypass")) {
> >> +			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
> >> +			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 0);
> >
> > matches is deparated, please use strcmp instead.
> 
> Why is strcmp recommended, not strncmp? I remember strcmp being frowned
> upon for some potential memory bounds violations.

We can't limit the string length as the parameter may have same prefix. e.g.
when you have 2 parameters "beef" and "beefsalad".

Hangbin
diff mbox series

Patch

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d61bd32d..fd390b40 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -824,6 +824,7 @@  enum {
 	IFLA_VXLAN_TTL_INHERIT,
 	IFLA_VXLAN_DF,
 	IFLA_VXLAN_VNIFILTER, /* only applicable with COLLECT_METADATA mode */
+	IFLA_VXLAN_LOCALBYPASS,
 	__IFLA_VXLAN_MAX
 };
 #define IFLA_VXLAN_MAX	(__IFLA_VXLAN_MAX - 1)
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index c7e0e1c4..17fa5cf7 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -276,6 +276,12 @@  static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 		} else if (!matches(*argv, "noudpcsum")) {
 			check_duparg(&attrs, IFLA_VXLAN_UDP_CSUM, *argv, *argv);
 			addattr8(n, 1024, IFLA_VXLAN_UDP_CSUM, 0);
+		} else if (!matches(*argv, "localbypass")) {
+			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
+			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 1);
+		} else if (!matches(*argv, "nolocalbypass")) {
+			check_duparg(&attrs, IFLA_VXLAN_LOCALBYPASS, *argv, *argv);
+			addattr8(n, 1024, IFLA_VXLAN_LOCALBYPASS, 0);
 		} else if (!matches(*argv, "udp6zerocsumtx")) {
 			check_duparg(&attrs, IFLA_VXLAN_UDP_ZERO_CSUM6_TX,
 				     *argv, *argv);
@@ -613,6 +619,18 @@  static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 	}
 
+	if (tb[IFLA_VXLAN_LOCALBYPASS]) {
+		__u8 localbypass = rta_getattr_u8(tb[IFLA_VXLAN_LOCALBYPASS]);
+
+		if (is_json_context()) {
+			print_bool(PRINT_ANY, "localbypass", NULL, localbypass);
+		} else {
+			if (!localbypass)
+				fputs("no", f);
+			fputs("localbypass ", f);
+		}
+	}
+
 	if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
 		__u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]);
 
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index c8c65657..c78dc9dd 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -623,6 +623,8 @@  the following additional arguments are supported:
 ] [
 .RB [ no ] udpcsum
 ] [
+.RB [ no ] localbypass
+] [
 .RB [ no ] udp6zerocsumtx
 ] [
 .RB [ no ] udp6zerocsumrx
@@ -727,6 +729,12 @@  are entered into the VXLAN device forwarding database.
 .RB [ no ] udpcsum
 - specifies if UDP checksum is calculated for transmitted packets over IPv4.
 
+.sp
+.RB [ no ] localbypass
+- if fdb destination is local, but there is no corresponding vni, forward packets
+to the userspace network stack. Supposedly, there may be a userspace process
+listening for these packets.
+
 .sp
 .RB [ no ] udp6zerocsumtx
 - skip UDP checksum calculation for transmitted packets over IPv6.