From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Jesper Dangaard Brouer <brouer@redhat.com>,
Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: David Miller <davem@davemloft.net>,
netdev@vger.kernel.org, Daniel Borkmann <daniel@iogearbox.net>,
Alexei Starovoitov <ast@kernel.org>,
brouer@redhat.com
Subject: Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
Date: Sat, 23 Feb 2019 13:11:02 +0100 [thread overview]
Message-ID: <875ztawvqh.fsf@toke.dk> (raw)
In-Reply-To: <20190223114343.5813f18a@carbon>
Jesper Dangaard Brouer <brouer@redhat.com> writes:
> On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>
>> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
>> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
> [...]
>> > >
>> > > BPF programs don't obey by netns boundaries. The fact the program is
>> > > verified in one ns doesn't mean this is the only ns it will be used in :(
>> > > Meaning if any program is using the redirect map you may need a secret
>> > > map in every ns.. no?
>> >
>> > Ah, yes, good point. Totally didn't think about the fact that load and
>> > attach are decoupled. Hmm, guess I'll just have to move the call to
>> > alloc_default_map() to the point where the program is attached to an
>> > interface, then...
>>
>> Possibly.. and you also need to handle the case where interface with a
>> program attached is moved, no?
Yup, alloc on attach was easy enough; the moving turns out to be the
tricky part :)
> True, we need to handle if e.g. a veth gets an XDP program attached and
> then is moved into a network namespace (as I've already explained to
> Toke in a meeting).
Yeah, I had somehow convinced myself that the XDP program was being
removed when the interface was being torn down before moving between
namespaces. Jesper pointed out that this was not in fact the case... :P
> I'm still not sure how to handle this...
There are a couple of options, I think. At least:
1. Maintain a flag on struct net_device indicating that this device
needs the redirect map allocated, and react to that when interfaces
are being moved.
2. Lookup the BPF program by ID (which we can get from the driver) on
move, and react to the program flag.
3. Keep the allocation on program load, but allocate maps for all active
namespaces (which would probably need a refcnt mechanism to
deallocate things again).
I think I'm leaning towards #2; possibly combined with a refcnt so we
can actually deallocate the map in the root namespace when it's not
needed anymore.
-Toke
next prev parent reply other threads:[~2019-02-23 12:11 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-21 11:56 [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device Toke Høiland-Jørgensen
2019-02-21 11:56 ` [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex Toke Høiland-Jørgensen
2019-02-21 15:23 ` Jesper Dangaard Brouer
2019-02-21 15:50 ` Toke Høiland-Jørgensen
2019-02-21 21:49 ` Jakub Kicinski
2019-02-21 23:02 ` Toke Høiland-Jørgensen
2019-02-22 0:32 ` Jakub Kicinski
2019-02-22 9:47 ` Toke Høiland-Jørgensen
2019-02-22 21:30 ` Jakub Kicinski
2019-02-23 11:52 ` Toke Høiland-Jørgensen
2019-02-23 23:19 ` kbuild test robot
2019-02-23 23:28 ` kbuild test robot
2019-02-21 15:19 ` [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device Jesper Dangaard Brouer
2019-02-21 15:52 ` Toke Høiland-Jørgensen
2019-02-22 0:36 ` Jakub Kicinski
2019-02-22 10:13 ` Toke Høiland-Jørgensen
2019-02-22 21:37 ` Jakub Kicinski
2019-02-23 10:43 ` Jesper Dangaard Brouer
2019-02-23 12:11 ` Toke Høiland-Jørgensen [this message]
2019-02-25 18:47 ` Jakub Kicinski
2019-02-26 11:00 ` Toke Høiland-Jørgensen
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=875ztawvqh.fsf@toke.dk \
--to=toke@redhat.com \
--cc=ast@kernel.org \
--cc=brouer@redhat.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=jakub.kicinski@netronome.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).