Re: bpf_fib_lookup support for firewall mark

[David Ahern <dsahern@xxxxxxxxx>]

On 6/10/21 2:58 PM, Daniel Borkmann wrote:
>> But, I do not think the vlan data should be overloaded right now. We
>> still have an open design issue around supporting vlans on ingress
>> (XDP). One option is to allow the lookup to take the vlan as an input,
>> have the the bpf helper lookup the vlan device that goes with the
>> {device index, vlan} pair and use that as the input device. If we
>> overload the vlan_TCI with fwmark that prohibits this option.
> 
> I guess it's not overly pretty, but if all things break down and there's
> no other
> unused space, wouldn't it work if we opt into the vlan as input (instead
> of mark)
> in future via flag?
> 
>>> Moreover, there are 12 extra bytes used only as output for the
>>> smac/dmac.
>>> If the above works then maybe this opens up the opportunity to
>>> incorporate
>>> even more input parameters that way?
>>
>> I think that's going to be tricky since the macs are 6-byte arrays.

This should work (whitespace damaged on paste). It preserves the vlan
for potential later use, and we have 2 more 4-byte holes. Untested, not
even compiled. Can you try it out?

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 418b9b813d65..476bc81f3d04 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5954,8 +5954,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 fwmark;
+                       /* 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 239de1306de9..a9b4fd2a6657 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5303,6 +5303,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->fwmark;
        fl4.flowi4_multipath_hash = 0;

        if (flags & BPF_FIB_LOOKUP_DIRECT) {
@@ -5315,7 +5316,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);
@@ -5429,6 +5429,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->fwmark;

        if (flags & BPF_FIB_LOOKUP_DIRECT) {
                u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
@@ -5441,7 +5442,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);