Re: [PATCH v10 04/12] rtnl: add option for setting link xdp prog

[Brenden Blanco]

On Wed, Jul 20, 2016 at 10:38:49AM +0200, Daniel Borkmann wrote:
> On 07/19/2016 09:16 PM, Brenden Blanco wrote:
> >Sets the bpf program represented by fd as an early filter in the rx path
> >of the netdev. The fd must have been created as BPF_PROG_TYPE_XDP.
> >Providing a negative value as fd clears the program. Getting the fd back
> >via rtnl is not possible, therefore reading of this value merely
> >provides a bool whether the program is valid on the link or not.
> >
> >Signed-off-by: Brenden Blanco <bbla...@plumgrid.com>
> [...]
> >@@ -2054,6 +2101,23 @@ static int do_setlink(const struct sk_buff *skb,
> >             status |= DO_SETLINK_NOTIFY;
> >     }
> >
> >+    if (tb[IFLA_XDP]) {
> >+            struct nlattr *xdp[IFLA_XDP_MAX + 1];
> >+
> >+            err = nla_parse_nested(xdp, IFLA_XDP_MAX, tb[IFLA_XDP],
> >+                                   ifla_xdp_policy);
> >+            if (err < 0)
> >+                    goto errout;
> >+
> >+            if (xdp[IFLA_XDP_FD]) {
> >+                    err = dev_change_xdp_fd(dev,
> >+                                            nla_get_s32(xdp[IFLA_XDP_FD]));
> >+                    if (err)
> >+                            goto errout;
> >+                    status |= DO_SETLINK_NOTIFY;
> >+            }
> 
> For the setlink case IFLA_XDP_ATTACHED has no meaning currently, so I'd
> suggest it would be good to be strict and still add a:
> 
>               /* Only used in rtnl_fill_ifinfo currently. */
>               if (xdp[IFLA_XDP_ATTACHED]) {
>                       err = -EINVAL;
>                       goto errout;
>               }
> 
Agreed.
> >+    }
> >+
> >  errout:
> >     if (status & DO_SETLINK_MODIFIED) {
> >             if (status & DO_SETLINK_NOTIFY)
> >
>