Tue, Oct 17, 2017 at 04:39:59PM CEST, jakub.kicin...@netronome.com wrote:
>On Tue, 17 Oct 2017 14:48:12 +0200, Jiri Pirko wrote:
>> Fri, Oct 13, 2017 at 03:08:24AM CEST, jakub.kicin...@netronome.com wrote:
>> >On Thu, 12 Oct 2017 19:18:16 +0200, Jiri Pirko wrote:
>> >> diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
>> >> b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
>> >> index a88bb5b..9e9af88 100644
>> >> --- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
>> >> +++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
>> >> @@ -246,6 +246,10 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct
>> >> tc_cls_bpf_offload *cls_bpf)
>> >> void *code;
>> >> int err;
>> >>
>> >> + if (cls_bpf->common.protocol != htons(ETH_P_ALL) ||
>> >> + cls_bpf->common.chain_index)
>> >> + return -EOPNOTSUPP;
>> >> +
>> >> max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
>> >>
>> >> switch (cls_bpf->command) {
>> >
>> >It is certainly very ugly but I send a fake struct tc_cls_bpf_offload
>> >here for XDP. Refactoring this mess is pretty high on my priority list
>> >but one way or the other this function will be called from XDP so TC
>> >checks must stay in the TC handler... :(
>>
>> Okay. But currently, why is it a problem? You don't need the checks for
>> xdp path.
>>
>
>static int
>nfp_bpf_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
> struct bpf_prog *prog)
>{
> struct tc_cls_bpf_offload cmd = {
> .prog = prog,
> };
> int ret;
>
> if (!nfp_net_ebpf_capable(nn))
> return -EINVAL;
>
> if (nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF) {
> if (!nn->dp.bpf_offload_xdp)
> return prog ? -EBUSY : 0;
> cmd.command = prog ? TC_CLSBPF_REPLACE : TC_CLSBPF_DESTROY;
> } else {
> if (!prog)
> return 0;
> cmd.command = TC_CLSBPF_ADD;
> }
>
> ret = nfp_net_bpf_offload(nn, &cmd);
> /* Stop offload if replace not possible */
> if (ret && cmd.command == TC_CLSBPF_REPLACE)
> nfp_bpf_xdp_offload(app, nn, NULL);
> nn->dp.bpf_offload_xdp = prog && !ret;
> return ret;
>}
>
>The fake offload struct is at the top of this function. Dereferencing
>cls_bpf->common in nfp_net_bpf_offload() will crash the kernel. Or am
>I missing something?
We just have to init it. Should not be a problem. Will add it.
