[PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Rumen Telbizov]

Add support for policy routing via marks to the bpf_fib_lookup
helper. The bpf_fib_lookup struct is constrained to 64B for
performance. Since the smac and dmac entries are used only for
output, put them in an anonymous struct and then add a union
around a second struct that contains the mark to use in the FIB
lookup.

Signed-off-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Rumen Telbizov <telbizov@gmail.com>
---
 include/uapi/linux/bpf.h | 16 ++++++++++++++--
 net/core/filter.c        |  4 ++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ec6d85a81744..6c78cc9c3c75 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5925,8 +5925,20 @@ struct bpf_fib_lookup {
  /* output */
  __be16 h_vlan_proto;
  __be16 h_vlan_TCI;
- __u8 smac[6];     /* ETH_ALEN */
- __u8 dmac[6];     /* ETH_ALEN */
+
+ union {
+ /* input */
+ struct {
+ __u32 mark;   /* fwmark for policy routing */
+ /* 2 4-byte holes for input */
+ };
+
+ /* output: source and dest mac */
+ struct {
+ __u8 smac[6]; /* ETH_ALEN */
+ __u8 dmac[6]; /* ETH_ALEN */
+ };
+ };
 };

 struct bpf_redir_neigh {
diff --git a/net/core/filter.c b/net/core/filter.c
index 65ab4e21c087..2ea997cacf4d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5299,6 +5299,7 @@ static int bpf_ipv4_fib_lookup(struct net *net,
struct bpf_fib_lookup *params,
  fl4.saddr = params->ipv4_src;
  fl4.fl4_sport = params->sport;
  fl4.fl4_dport = params->dport;
+ fl4.flowi4_mark = params->mark;
  fl4.flowi4_multipath_hash = 0;

  if (flags & BPF_FIB_LOOKUP_DIRECT) {
@@ -5311,7 +5312,6 @@ static int bpf_ipv4_fib_lookup(struct net *net,
struct bpf_fib_lookup *params,

  err = fib_table_lookup(tb, &fl4, &res, FIB_LOOKUP_NOREF);
  } else {
- fl4.flowi4_mark = 0;
  fl4.flowi4_secid = 0;
  fl4.flowi4_tun_key.tun_id = 0;
  fl4.flowi4_uid = sock_net_uid(net, NULL);
@@ -5425,6 +5425,7 @@ static int bpf_ipv6_fib_lookup(struct net *net,
struct bpf_fib_lookup *params,
  fl6.saddr = *src;
  fl6.fl6_sport = params->sport;
  fl6.fl6_dport = params->dport;
+ fl6.flowi6_mark = params->mark;

  if (flags & BPF_FIB_LOOKUP_DIRECT) {
  u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
@@ -5437,7 +5438,6 @@ static int bpf_ipv6_fib_lookup(struct net *net,
struct bpf_fib_lookup *params,
  err = ipv6_stub->fib6_table_lookup(net, tb, oif, &fl6, &res,
    strict);
  } else {
- fl6.flowi6_mark = 0;
  fl6.flowi6_secid = 0;
  fl6.flowi6_tun_key.tun_id = 0;
  fl6.flowi6_uid = sock_net_uid(net, NULL);
--
2.30.1 (Apple Git-130)

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Greg KH]

On Tue, Jun 29, 2021 at 10:37:34AM -0700, Rumen Telbizov wrote:
> Add support for policy routing via marks to the bpf_fib_lookup
> helper. The bpf_fib_lookup struct is constrained to 64B for
> performance. Since the smac and dmac entries are used only for
> output, put them in an anonymous struct and then add a union
> around a second struct that contains the mark to use in the FIB
> lookup.
> 
> Signed-off-by: David Ahern <dsahern@kernel.org>
> Signed-off-by: Rumen Telbizov <telbizov@gmail.com>
> ---
>  include/uapi/linux/bpf.h | 16 ++++++++++++++--
>  net/core/filter.c        |  4 ++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index ec6d85a81744..6c78cc9c3c75 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5925,8 +5925,20 @@ struct bpf_fib_lookup {
>   /* output */
>   __be16 h_vlan_proto;
>   __be16 h_vlan_TCI;
> - __u8 smac[6];     /* ETH_ALEN */
> - __u8 dmac[6];     /* ETH_ALEN */
> +
> + union {
> + /* input */
> + struct {
> + __u32 mark;   /* fwmark for policy routing */
> + /* 2 4-byte holes for input */
> + };

Tabs seem to be eaten and spit back out with spaces from your email
client :(

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Greg KH]

On Tue, Jun 29, 2021 at 10:37:34AM -0700, Rumen Telbizov wrote:
> Add support for policy routing via marks to the bpf_fib_lookup
> helper. The bpf_fib_lookup struct is constrained to 64B for
> performance. Since the smac and dmac entries are used only for
> output, put them in an anonymous struct and then add a union
> around a second struct that contains the mark to use in the FIB
> lookup.
> 
> Signed-off-by: David Ahern <dsahern@kernel.org>
> Signed-off-by: Rumen Telbizov <telbizov@gmail.com>

Did David author this, or did Rumen?

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[David Ahern]

On 6/29/21 11:50 AM, Greg KH wrote:
> On Tue, Jun 29, 2021 at 10:37:34AM -0700, Rumen Telbizov wrote:
>> Add support for policy routing via marks to the bpf_fib_lookup
>> helper. The bpf_fib_lookup struct is constrained to 64B for
>> performance. Since the smac and dmac entries are used only for
>> output, put them in an anonymous struct and then add a union
>> around a second struct that contains the mark to use in the FIB
>> lookup.
>>
>> Signed-off-by: David Ahern <dsahern@kernel.org>
>> Signed-off-by: Rumen Telbizov <telbizov@gmail.com>
> 
> Did David author this, or did Rumen?
> 

Both. Rumen's first patch set and hitting some bumps.

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Toke Høiland-Jørgensen]

Rumen Telbizov <rumen.telbizov@menlosecurity.com> writes:

> Add support for policy routing via marks to the bpf_fib_lookup
> helper. The bpf_fib_lookup struct is constrained to 64B for
> performance. Since the smac and dmac entries are used only for
> output, put them in an anonymous struct and then add a union
> around a second struct that contains the mark to use in the FIB
> lookup.
>
> Signed-off-by: David Ahern <dsahern@kernel.org>
> Signed-off-by: Rumen Telbizov <telbizov@gmail.com>
> ---
>  include/uapi/linux/bpf.h | 16 ++++++++++++++--
>  net/core/filter.c        |  4 ++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index ec6d85a81744..6c78cc9c3c75 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5925,8 +5925,20 @@ struct bpf_fib_lookup {
>   /* output */
>   __be16 h_vlan_proto;
>   __be16 h_vlan_TCI;
> - __u8 smac[6];     /* ETH_ALEN */
> - __u8 dmac[6];     /* ETH_ALEN */
> +
> + union {
> + /* input */
> + struct {
> + __u32 mark;   /* fwmark for policy routing */
> + /* 2 4-byte holes for input */
> + };
> +
> + /* output: source and dest mac */
> + struct {
> + __u8 smac[6]; /* ETH_ALEN */
> + __u8 dmac[6]; /* ETH_ALEN */
> + };
> + };
>  };

Looks like your mailer mangled the indentation of the whole thing.
Threading is broken too...

-Toke

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Rumen Telbizov]

Give credit to David Ahern for this patch. Shall we change anything there?

Sorry for the whitespaces. It seems like I can't properly do this from gmail.
Let me try to redo the whole thing from the command line.

On Tue, Jun 29, 2021 at 10:51 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Rumen Telbizov <rumen.telbizov@menlosecurity.com> writes:
>
> > Add support for policy routing via marks to the bpf_fib_lookup
> > helper. The bpf_fib_lookup struct is constrained to 64B for
> > performance. Since the smac and dmac entries are used only for
> > output, put them in an anonymous struct and then add a union
> > around a second struct that contains the mark to use in the FIB
> > lookup.
> >
> > Signed-off-by: David Ahern <dsahern@kernel.org>
> > Signed-off-by: Rumen Telbizov <telbizov@gmail.com>
> > ---
> > include/uapi/linux/bpf.h | 16 ++++++++++++++--
> > net/core/filter.c | 4 ++--
> > 2 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index ec6d85a81744..6c78cc9c3c75 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -5925,8 +5925,20 @@ struct bpf_fib_lookup {
> > /* output */
> > __be16 h_vlan_proto;
> > __be16 h_vlan_TCI;
> > - __u8 smac[6]; /* ETH_ALEN */
> > - __u8 dmac[6]; /* ETH_ALEN */
> > +
> > + union {
> > + /* input */
> > + struct {
> > + __u32 mark; /* fwmark for policy routing */
> > + /* 2 4-byte holes for input */
> > + };
> > +
> > + /* output: source and dest mac */
> > + struct {
> > + __u8 smac[6]; /* ETH_ALEN */
> > + __u8 dmac[6]; /* ETH_ALEN */
> > + };
> > + };
> > };
>
> Looks like your mailer mangled the indentation of the whole thing.
> Threading is broken too...
>
> -Toke
>

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Greg KH]

On Tue, Jun 29, 2021 at 11:06:14AM -0700, Rumen Telbizov wrote:
> Give credit to David Ahern for this patch. Shall we change anything there?

Put him as the From: line like the documentation suggests for stuff like
this.

or use git send-email, it will handle it for you automatically.

thanks,

greg k-h

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Rumen Telbizov]

Ok, I'll do that.

Sorry for the confusion. My first time trying to submit a patch.
Wrangling with git to do it the right way now. Please ignore these for now.
I will submit a new one shortly.

On Tue, Jun 29, 2021 at 11:13 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 29, 2021 at 11:06:14AM -0700, Rumen Telbizov wrote:
> > Give credit to David Ahern for this patch. Shall we change anything there?
>
> Put him as the From: line like the documentation suggests for stuff like
> this.
>
> or use git send-email, it will handle it for you automatically.
>
> thanks,
>
> greg k-h

[PATCH 0/3] Add support for fwmark to bpf_fib_lookup

[Rumen Telbizov]

Add support for fwmark to bpf_fib_lookup.

Patch 1: adds the support to the uapi and updates the lookup helper.
Patch 2: copies the uapi change to the tools directory.
Patch 3: adds test cases.

Rumen Telbizov (3):
  bpf: Add support for mark with bpf_fib_lookup
  tools: Update bpf header
  selftests: Add selftests for fwmark support in bpf_fib_lookup

 include/uapi/linux/bpf.h                      |  16 +-
 net/core/filter.c                             |   4 +-
 tools/include/uapi/linux/bpf.h                |  16 +-
 tools/testing/selftests/bpf/Makefile          |   1 +
 .../selftests/bpf/progs/test_bpf_fib_lookup.c | 135 ++++++++++++++
 .../selftests/bpf/test_bpf_fib_lookup.sh      | 166 ++++++++++++++++++
 6 files changed, 332 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_fib_lookup.c
 create mode 100755 tools/testing/selftests/bpf/test_bpf_fib_lookup.sh

-- 
2.30.1 (Apple Git-130)

[PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Rumen Telbizov]

From: David Ahern <dsahern@kernel.org>

Add support for policy routing via marks to the bpf_fib_lookup
helper. The bpf_fib_lookup struct is constrained to 64B for
performance. Since the smac and dmac entries are used only for
output, put them in an anonymous struct and then add a union
around a second struct that contains the mark to use in the FIB
lookup.

Signed-off-by: Rumen Telbizov <rumen.telbizov@menlosecurity.com>
---
 include/uapi/linux/bpf.h | 16 ++++++++++++++--
 net/core/filter.c        |  4 ++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ec6d85a81744..6c78cc9c3c75 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5925,8 +5925,20 @@ struct bpf_fib_lookup {
 	/* output */
 	__be16	h_vlan_proto;
 	__be16	h_vlan_TCI;
-	__u8	smac[6];     /* ETH_ALEN */
-	__u8	dmac[6];     /* ETH_ALEN */
+
+	union {
+		/* input */
+		struct {
+			__u32	mark;   /* fwmark for policy routing */
+			/* 2 4-byte holes for input */
+		};
+
+		/* output: source and dest mac */
+		struct {
+			__u8	smac[6];	/* ETH_ALEN */
+			__u8	dmac[6];	/* ETH_ALEN */
+		};
+	};
 };
 
 struct bpf_redir_neigh {
diff --git a/net/core/filter.c b/net/core/filter.c
index 65ab4e21c087..2ea997cacf4d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5299,6 +5299,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	fl4.saddr = params->ipv4_src;
 	fl4.fl4_sport = params->sport;
 	fl4.fl4_dport = params->dport;
+	fl4.flowi4_mark = params->mark;
 	fl4.flowi4_multipath_hash = 0;
 
 	if (flags & BPF_FIB_LOOKUP_DIRECT) {
@@ -5311,7 +5312,6 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 
 		err = fib_table_lookup(tb, &fl4, &res, FIB_LOOKUP_NOREF);
 	} else {
-		fl4.flowi4_mark = 0;
 		fl4.flowi4_secid = 0;
 		fl4.flowi4_tun_key.tun_id = 0;
 		fl4.flowi4_uid = sock_net_uid(net, NULL);
@@ -5425,6 +5425,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	fl6.saddr = *src;
 	fl6.fl6_sport = params->sport;
 	fl6.fl6_dport = params->dport;
+	fl6.flowi6_mark = params->mark;
 
 	if (flags & BPF_FIB_LOOKUP_DIRECT) {
 		u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
@@ -5437,7 +5438,6 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 		err = ipv6_stub->fib6_table_lookup(net, tb, oif, &fl6, &res,
 						   strict);
 	} else {
-		fl6.flowi6_mark = 0;
 		fl6.flowi6_secid = 0;
 		fl6.flowi6_tun_key.tun_id = 0;
 		fl6.flowi6_uid = sock_net_uid(net, NULL);
-- 
2.30.1 (Apple Git-130)

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[Greg KH]

On Tue, Jun 29, 2021 at 11:55:35AM -0700, Rumen Telbizov wrote:
> From: David Ahern <dsahern@kernel.org>
> 
> Add support for policy routing via marks to the bpf_fib_lookup
> helper. The bpf_fib_lookup struct is constrained to 64B for
> performance. Since the smac and dmac entries are used only for
> output, put them in an anonymous struct and then add a union
> around a second struct that contains the mark to use in the FIB
> lookup.
> 
> Signed-off-by: Rumen Telbizov <rumen.telbizov@menlosecurity.com>
> ---

Any reason that David didn't also sign off on this?

thanks,

greg k-h

Re: [PATCH 1/3] bpf: Add support for mark with bpf_fib_lookup

[David Ahern]

On 6/29/21 11:35 PM, Greg KH wrote:
> On Tue, Jun 29, 2021 at 11:55:35AM -0700, Rumen Telbizov wrote:
>> From: David Ahern <dsahern@kernel.org>
>>
>> Add support for policy routing via marks to the bpf_fib_lookup
>> helper. The bpf_fib_lookup struct is constrained to 64B for
>> performance. Since the smac and dmac entries are used only for
>> output, put them in an anonymous struct and then add a union
>> around a second struct that contains the mark to use in the FIB
>> lookup.
>>
>> Signed-off-by: Rumen Telbizov <rumen.telbizov@menlosecurity.com>
>> ---
> 
> Any reason that David didn't also sign off on this?
> 

I did; just confusion by a first timer in taking my patch and adding his
patches to it. It will be fixed in v2.