diff mbox series

[net,05/10] net: xdp: don't allow device-bound programs in driver mode

Message ID 20171120045522.2188-6-jakub.kicinski@netronome.com
State Changes Requested, archived
Delegated to: BPF Maintainers
Headers show
Series bpf: offload: check netdev pointer in the drivers and namespace trouble | expand

Commit Message

Jakub Kicinski Nov. 20, 2017, 4:55 a.m. UTC 
Currently device-bound programs are not able to run on the host
to save resources (host JIT is not invoked).  Don't allow XDP
programs to be attached without the HW_MODE flag.  In theory
if program is already translated for device offload the driver
should choose to offload it instead of loading it in the driver.
However, offloading translated program may still fail resulting
in device-bound program being run on the host.

Prevent this by refusing to attach device bound programs if
XDP_FLAGS_HW_MODE is not set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/core/dev.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

David Ahern Nov. 20, 2017, 2:36 p.m. UTC | #1 
On 11/19/17 9:55 PM, Jakub Kicinski wrote:
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 09525a27319c..21de2d37a0ba 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7143,6 +7143,13 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
>  					     bpf_op == ops->ndo_bpf);
>  		if (IS_ERR(prog))
>  			return PTR_ERR(prog);
> +
> +		if (!(flags & XDP_FLAGS_HW_MODE) &&
> +		    bpf_prog_is_dev_bound(prog->aux)) {
> +			NL_SET_ERR_MSG_MOD(extack, "using device-bound program without HW_MODE flag not supported");

I don't see dev_change_xdp_fd called by device drivers, so that should
just be NL_SET_ERR_MSG. Also, "is not supported" sounds better to me
than just "not supported".
Jakub Kicinski Nov. 20, 2017, 10:02 p.m. UTC | #2 
On Mon, 20 Nov 2017 07:36:39 -0700, David Ahern wrote:
> On 11/19/17 9:55 PM, Jakub Kicinski wrote:
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 09525a27319c..21de2d37a0ba 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -7143,6 +7143,13 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
> >  					     bpf_op == ops->ndo_bpf);
> >  		if (IS_ERR(prog))
> >  			return PTR_ERR(prog);
> > +
> > +		if (!(flags & XDP_FLAGS_HW_MODE) &&
> > +		    bpf_prog_is_dev_bound(prog->aux)) {
> > +			NL_SET_ERR_MSG_MOD(extack, "using device-bound program without HW_MODE flag not supported");  
> 
> I don't see dev_change_xdp_fd called by device drivers, so that should
> just be NL_SET_ERR_MSG. Also, "is not supported" sounds better to me
> than just "not supported".

Thanks, I'll give others a couple more hours and respin!
Daniel Borkmann Nov. 20, 2017, 10:21 p.m. UTC | #3 
Hi Jakub,

On 11/20/2017 11:02 PM, Jakub Kicinski wrote:
> On Mon, 20 Nov 2017 07:36:39 -0700, David Ahern wrote:
>> On 11/19/17 9:55 PM, Jakub Kicinski wrote:
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 09525a27319c..21de2d37a0ba 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -7143,6 +7143,13 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
>>>  					     bpf_op == ops->ndo_bpf);
>>>  		if (IS_ERR(prog))
>>>  			return PTR_ERR(prog);
>>> +
>>> +		if (!(flags & XDP_FLAGS_HW_MODE) &&
>>> +		    bpf_prog_is_dev_bound(prog->aux)) {
>>> +			NL_SET_ERR_MSG_MOD(extack, "using device-bound program without HW_MODE flag not supported");  
>>
>> I don't see dev_change_xdp_fd called by device drivers, so that should
>> just be NL_SET_ERR_MSG. Also, "is not supported" sounds better to me
>> than just "not supported".
> 
> Thanks, I'll give others a couple more hours and respin!

The rest of the series looks good to me, please respin with the minor
changes requested.

Thanks,
Daniel
diff mbox series

Patch

diff --git a/net/core/dev.c b/net/core/dev.c
index 09525a27319c..21de2d37a0ba 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7143,6 +7143,13 @@  int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 					     bpf_op == ops->ndo_bpf);
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
+
+		if (!(flags & XDP_FLAGS_HW_MODE) &&
+		    bpf_prog_is_dev_bound(prog->aux)) {
+			NL_SET_ERR_MSG_MOD(extack, "using device-bound program without HW_MODE flag not supported");
+			bpf_prog_put(prog);
+			return -EINVAL;
+		}
 	}
 
 	err = dev_xdp_install(dev, bpf_op, extack, flags, prog);