From: Daniel Borkmann <daniel@iogearbox•net>
To: Jakub Kicinski <kubakici@wp•pl>
Cc: netdev@vger•kernel.org, davem@davemloft•net, kafai@fb•com,
brouer@redhat•com, oss-drivers@netronome•com
Subject: Re: [RFC net-next 6/8] nfp: bpf: add support for XDP_FLAGS_HW_MODE
Date: Tue, 20 Jun 2017 02:09:54 +0200 [thread overview]
Message-ID: <594867D2.4030306@iogearbox.net> (raw)
In-Reply-To: <20170619170123.1fc13a70@cakuba.netronome.com>
On 06/20/2017 02:01 AM, Jakub Kicinski wrote:
> On Tue, 20 Jun 2017 01:50:17 +0200, Daniel Borkmann wrote:
>> On 06/17/2017 01:57 AM, Jakub Kicinski wrote:
>>> Respect the XDP_FLAGS_HW_MODE. When it's set install the program
>>> on the NIC and skip enabling XDP in the driver.
>>>
>>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome•com>
>>> ---
>>> drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 10 +++++++---
>>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
>>> index 68648e312129..c5903b6e58c5 100644
>>> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
>>> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
>>> @@ -3310,19 +3310,22 @@ static int
>>> nfp_net_xdp_setup(struct nfp_net *nn, struct bpf_prog *prog, u32 flags,
>>> struct netlink_ext_ack *extack)
>>> {
>>> - struct bpf_prog *offload_prog;
>>> + struct bpf_prog *drv_prog, *offload_prog;
>>> int err;
>>>
>>> if (nn->xdp_prog && (flags ^ nn->xdp_flags) & XDP_FLAGS_MODES)
>>> return -EBUSY;
>>>
>>> + drv_prog = flags & XDP_FLAGS_HW_MODE ? NULL : prog;
>>> offload_prog = flags & XDP_FLAGS_DRV_MODE ? NULL : prog;
>>
>> Can you make this assumption here? If dev_change_xdp_fd() is called
>> without XDP_FLAGS_HW_MODE or XDP_FLAGS_DRV_MODE flags, then we set prog
>> to both, drv_prog and offload_prog. Is this expected?
>>
>> Maybe in nfp_net_xdp_setup() check for !hweight32(xdp_flags & XDP_FLAGS_MODES)
>> and then set flags |= XDP_FLAGS_DRV_MODE before both assignments?
>
> I thought we did want both. In case the program is loaded to both the
> HW/FW will mark the packets with BPF bit in the descriptor so that they
> are not processed twice. But the driver path will be configured for
> running bpf and when user replaces the program with one which cannot be
> offloaded the driver will not have to reconfigure itself.
Okay, that's a good point ... so that you can just use xchg() later on.
Probably worth explaining this rationale in a short comment.
next prev parent reply other threads:[~2017-06-20 0:09 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-16 23:57 [RFC net-next 0/8] xdp: offload mode Jakub Kicinski
2017-06-16 23:57 ` [RFC net-next 1/8] xdp: pass XDP flags into install handlers Jakub Kicinski
2017-06-19 23:38 ` Daniel Borkmann
2017-06-16 23:57 ` [RFC net-next 2/8] xdp: add HW offload mode flag for installing programs Jakub Kicinski
2017-06-19 22:55 ` Daniel Borkmann
2017-06-19 23:24 ` Jakub Kicinski
2017-06-19 23:36 ` Daniel Borkmann
2017-06-19 23:39 ` Daniel Borkmann
2017-06-16 23:57 ` [RFC net-next 3/8] nfp: xdp: move driver XDP setup into a separate function Jakub Kicinski
2017-06-16 23:57 ` [RFC net-next 4/8] nfp: bpf: don't offload XDP programs in DRV_MODE Jakub Kicinski
2017-06-16 23:57 ` [RFC net-next 5/8] nfp: bpf: take a reference on offloaded programs Jakub Kicinski
2017-06-19 23:23 ` Daniel Borkmann
2017-06-19 23:34 ` Jakub Kicinski
2017-06-16 23:57 ` [RFC net-next 6/8] nfp: bpf: add support for XDP_FLAGS_HW_MODE Jakub Kicinski
2017-06-19 23:50 ` Daniel Borkmann
2017-06-20 0:01 ` Jakub Kicinski
2017-06-20 0:09 ` Daniel Borkmann [this message]
2017-06-16 23:57 ` [RFC net-next 7/8] xdp: add reporting of offload mode Jakub Kicinski
2017-06-19 23:40 ` Daniel Borkmann
2017-06-16 23:57 ` [RFC net-next 8/8] nfp: xdp: report if program is offloaded Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=594867D2.4030306@iogearbox.net \
--to=daniel@iogearbox$(echo .)net \
--cc=brouer@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=kafai@fb$(echo .)com \
--cc=kubakici@wp$(echo .)pl \
--cc=netdev@vger$(echo .)kernel.org \
--cc=oss-drivers@netronome$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox