* Re: [PATCH v2 bpf-next 00/11] BTF-defined BPF map definitions
From: Lorenz Bauer @ 2019-06-20 14:49 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Daniel Borkmann, Andrii Nakryiko, Alexei Starovoitov, Networking,
bpf, Kernel Team, Jakub Kicinski, Joe Stringer
In-Reply-To: <CAEf4Bzae1CPDkhPrESa2ZmiOH8Mqf0KA_4ty9z=xnYn=q7Frhw@mail.gmail.com>
On Tue, 18 Jun 2019 at 22:37, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > I would just drop the object-scope pinning. We avoided using it and I'm not
> > aware if anyone else make use. It also has the ugly side-effect that this
> > relies on AF_ALG which e.g. on some cloud provider shipped kernels is disabled.
> > The pinning attribute should be part of the standard set of map attributes for
> > libbpf though as it's generally useful for networking applications.
>
> Sounds good. I'll do some more surveying of use cases inside FB to see
> if anyone needs object-scope pinning, just to be sure we are not
> short-cutting anyone.
I'm also curious what the use cases for declarative pinning are. From my
limited POV it doesn't seem that useful? There are a couple of factors:
* Systemd mounts the default location only accessible to root, so I have to
used my own bpffs mount.
* Since I don't want to hard code that, I put it in a config file.
* After loading the ELF we pin maps from the daemon managing the XDP.
How do other people work around this? Hard coding it in the ELF seems
suboptimal.
> > And the loader should figure this out and combine everything in the background.
> > Otherwise above 'struct inner_map_t value' would be mixing convention of using
> > pointer vs non-pointer which may be even more confusing.
>
> There are two reasons I didn't want to go with that approach:
>
> 1. This syntax makes my_inner_map usable as a stand-alone map, while
> it's purpose is to serve as a inner map prototype. While technically
> it is ok to use my_inner_map as real map, it's kind of confusing and
> feels unclean.
I agree, avoiding this problem is good.
> So we came up with a way to "encode" integer constants as part of BTF
> type information, so that *all* declarative information is part of BTF
> type, w/o the need to compile-time initialization. We tried to go the
> other way (what Jakub was pushing for), but we couldn't figure out
> anything that would work w/o more compiler hacks. So here's the
> updated proposal:
>
> #define __int(name, val) int (*name)[val]
Consider my mind blown: https://cdecl.org/?q=int+%28*foo%29%5B10%5D
> #define __type(name, val) val (*foo)
Maybe it's enough to just hide the pointer-ness?
#define __member(name) (*name)
struct my_value __member(value);
> struct my_inner_map {
> __int(type, BPF_MAP_TYPE_ARRAY);
> __int(max_entries, 1000);
> __type(key, int);
> __type(value, struct my_value);
What if this did
__type(value, struct my_value)[1000];
struct my_value __member(value)[1000]; // alternative
instead, and skipped max_entries?
> static struct {
> __int(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
> __int(max_entries, 1000);
> __type(key, int);
> __type(value, struct my_inner_map);
> struct my_inner_map *values[];
> } my_initialized_outer_map SEC(".maps") = {
> .values = {
> &imap1,
> [500] = &imap2,
> },
> };
>
> Here struct my_inner_map is complete definition of array map w/ 1000
> elements w/ all the type info for k/v. That struct is used as a
> template for my_outer_map map-in-map. my_initialized_outer_map is the
> case of pre-initialization of array-of-maps w/ instances of existing
> maps imap1 and imap2.
For my_initialized_outer_map, which section does .values end up in the
generated ELF? How much space is going to be allocated? 501 * 4 bytes?
> The idea is that we encode integer fields as array dimensions + use
> pointer to an array to save space. Given that syntax in plain C is a
> bit ugly and hard to remember, we hide that behind __int macro. Then
> in line with __int, we also have __type macro, that hides that hateful
> pointer for key/value types. This allows map definition to be
> self-describing w/o having to look at initialized ELF data section at
> all, except for special cases of explicitly initializing map-in-map or
> prog_array.
>
> What do you think?
I think this is an interesting approach. One thing I'm not sure of is handling
these types from C. For example:
sizeof(my_outer_map.value)
This compiles, but doesn't produce the intended result. Correct would be:
sizeof(my_outer_map.value[0])
At that point you have to understand that value is a pointer so all of
our efforts
are for naught. I suspect there is other weirdness like this, but I need to play
with it a little bit more.
> Yeah I can definitely see some confusion here. But it seems like this
> is more of a semantics of map sharing, and maybe it should be some
> extra option for when we have automatic support for extern (shared)
> maps. E.g., something like
>
> __int(sharing, SHARE_STRATEGY_MERGE) vs __int(sharing, SHARE_STRATEGY_OVERWRITE)
>
> Haven't though through exact syntax, naming, semantics, but it seems
> doable to support both, depending on desired behavior.
>
> Maybe we should also unify this w/ pinning? E.g., there are many
> sensible ways to handle already existing pinned map:
>
> 1. Reject program (e.g., if BPF application is the source of truth for that map)
> 2. Use pinned as is (e.g., if BPF application wants to consume data
> from source of truth app)
> 3. Merge (what you described above)
> 4. Replace/reset - not sure if useful/desirable.
From my experience, trying to support many use cases in a purely declarative
fashion ends up creating many edge cases, and quirky behaviour that is hard to
fix later on. It's a bit like merging dictionaries in $LANGUAGE,
which starts out simple and then gets complicated because sometimes you
want to override a key, but lists should be concatenated, except in
that one case...
I wonder: are there many use cases where writing some glue code isn't
possible? With libbpf getting more mature APIs that should become easier and
easier. We could probably support existing iproute2 features that way as well.
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
www.cloudflare.com
^ permalink raw reply
* [PATCH -next] inet: fix compilation warnings in fqdir_pre_exit()
From: Qian Cai @ 2019-06-20 14:52 UTC (permalink / raw)
To: davem; +Cc: edumazet, netdev, linux-kernel, Qian Cai
The linux-next commit "inet: fix various use-after-free in defrags
units" [1] introduced compilation warnings,
./include/net/inet_frag.h:117:1: warning: 'inline' is not at beginning
of declaration [-Wold-style-declaration]
static void inline fqdir_pre_exit(struct fqdir *fqdir)
^~~~~~
In file included from ./include/net/netns/ipv4.h:10,
from ./include/net/net_namespace.h:20,
from ./include/linux/netdevice.h:38,
from ./include/linux/icmpv6.h:13,
from ./include/linux/ipv6.h:86,
from ./include/net/ipv6.h:12,
from ./include/rdma/ib_verbs.h:51,
from ./include/linux/mlx5/device.h:37,
from ./include/linux/mlx5/driver.h:51,
from
drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:37:
[1] https://lore.kernel.org/netdev/20190618180900.88939-3-edumazet@google.com/
Signed-off-by: Qian Cai <cai@lca.pw>
---
include/net/inet_frag.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index 46574d996f1d..010f26b31c89 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -114,7 +114,7 @@ struct inet_frags {
int fqdir_init(struct fqdir **fqdirp, struct inet_frags *f, struct net *net);
-static void inline fqdir_pre_exit(struct fqdir *fqdir)
+static inline void fqdir_pre_exit(struct fqdir *fqdir)
{
fqdir->high_thresh = 0; /* prevent creation of new frags */
fqdir->dead = true;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] ipv6: fix neighbour resolution with raw socket
From: David Ahern @ 2019-06-20 15:12 UTC (permalink / raw)
To: Nicolas Dichtel, davem; +Cc: netdev
In-Reply-To: <20190620123434.7219-1-nicolas.dichtel@6wind.com>
On 6/20/19 6:34 AM, Nicolas Dichtel wrote:
> The scenario is the following: the user uses a raw socket to send an ipv6
> packet, destinated to a not-connected network, and specify a connected nh.
> Here is the corresponding python script to reproduce this scenario:
>
> import socket
> IPPROTO_RAW = 255
> send_s = socket.socket(socket.AF_INET6, socket.SOCK_RAW, IPPROTO_RAW)
> # scapy
> # p = IPv6(src='fd00:100::1', dst='fd00:200::fa')/ICMPv6EchoRequest()
> # str(p)
> req = b'`\x00\x00\x00\x00\x08:@\xfd\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\xfd\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xfa\x80\x00\x81\xc0\x00\x00\x00\x00'
> send_s.sendto(req, ('fd00:175::2', 0, 0, 0))
>
> fd00:175::/64 is a connected route and fd00:200::fa is not a connected
> host.
>
> With this scenario, the kernel starts by sending a NS to resolve
> fd00:175::2. When it receives the NA, it flushes its queue and try to send
> the initial packet. But instead of sending it, it sends another NS to
> resolve fd00:200::fa, which obvioulsy fails, thus the packet is dropped. If
> the user sends again the packet, it now uses the right nh (fd00:175::2).
>
what's the local address and route setup? You reference fd00:100::1 and
fd00:200::fa with connected route fd00:175::/64.
^ permalink raw reply
* Re: [PATCH net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Willem de Bruijn @ 2019-06-20 15:16 UTC (permalink / raw)
To: Neil Horman; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <20190620142354.GB18890@hmswarspite.think-freely.org>
On Thu, Jun 20, 2019 at 10:24 AM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Thu, Jun 20, 2019 at 09:41:30AM -0400, Willem de Bruijn wrote:
> > On Wed, Jun 19, 2019 at 4:26 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > When an application is run that:
> > > a) Sets its scheduler to be SCHED_FIFO
> > > and
> > > b) Opens a memory mapped AF_PACKET socket, and sends frames with the
> > > MSG_DONTWAIT flag cleared, its possible for the application to hang
> > > forever in the kernel. This occurs because when waiting, the code in
> > > tpacket_snd calls schedule, which under normal circumstances allows
> > > other tasks to run, including ksoftirqd, which in some cases is
> > > responsible for freeing the transmitted skb (which in AF_PACKET calls a
> > > destructor that flips the status bit of the transmitted frame back to
> > > available, allowing the transmitting task to complete).
> > >
> > > However, when the calling application is SCHED_FIFO, its priority is
> > > such that the schedule call immediately places the task back on the cpu,
> > > preventing ksoftirqd from freeing the skb, which in turn prevents the
> > > transmitting task from detecting that the transmission is complete.
> > >
> > > We can fix this by converting the schedule call to a completion
> > > mechanism. By using a completion queue, we force the calling task, when
> > > it detects there are no more frames to send, to schedule itself off the
> > > cpu until such time as the last transmitted skb is freed, allowing
> > > forward progress to be made.
> > >
> > > Tested by myself and the reporter, with good results
> > >
> > > Appies to the net tree
> > >
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > Reported-by: Matteo Croce <mcroce@redhat.com>
> > > CC: "David S. Miller" <davem@davemloft.net>
> > > ---
> >
> > This is a complex change for a narrow configuration. Isn't a
> > SCHED_FIFO process preempting ksoftirqd a potential problem for other
> > networking workloads as well? And the right configuration to always
> > increase ksoftirqd priority when increasing another process's
> > priority? Also, even when ksoftirqd kicks in, isn't some progress
> > still made on the local_bh_enable reached from schedule()?
> >
>
> A few questions here to answer:
Thanks for the detailed explanation.
> Regarding other protocols having this problem, thats not the case, because non
> packet sockets honor the SK_SNDTIMEO option here (i.e. they sleep for a period
> of time specified by the SNDTIMEO option if MSG_DONTWAIT isn't set. We could
> certainly do that, but the current implementation doesn't (opting instead to
> wait indefinately until the respective packet(s) have transmitted or errored
> out), and I wanted to maintain that behavior. If there is consensus that packet
> sockets should honor SNDTIMEO, then I can certainly do that.
>
> As for progress made by calling local_bh_enable, My read of the code doesn't
> have the scheduler calling local_bh_enable at all. Instead schedule uses
> preempt_disable/preempt_enable_no_resched() to gain exlcusive access to the cpu,
> which ignores pending softirqs on re-enablement.
Ah, I'm mistaken there, then.
> Perhaps that needs to change,
> but I'm averse to making scheduler changes for this (the aforementioned concern
> about complex changes for a narrow use case)
>
> Regarding raising the priority of ksoftirqd, that could be a solution, but the
> priority would need to be raised to a high priority SCHED_FIFO parameter, and
> that gets back to making complex changes for a narrow problem domain
>
> As for the comlexity of the of the solution, I think this is, given your
> comments the least complex and intrusive change to solve the given problem.
Could it be simpler to ensure do_softirq() gets run here? That would
allow progress for this case.
> We
> need to find a way to force the calling task off the cpu while the asynchronous
> operations in the transmit path complete, and we can do that this way, or by
> honoring SK_SNDTIMEO. I'm fine with doing the latter, but I didn't want to
> alter the current protocol behavior without consensus on that.
In general SCHED_FIFO is dangerous with regard to stalling other
progress, incl. ksoftirqd. But it does appear that this packet socket
case is special inside networking in calling schedule() directly here.
If converting that, should it convert to logic more akin to other
sockets, like sock_wait_for_wmem? I haven't had a chance to read up on
the pros and cons of completion here yet, sorry. Didn't want to delay
responding until after I get a chance.
^ permalink raw reply
* Re: [PATCH -next] inet: fix compilation warnings in fqdir_pre_exit()
From: Eric Dumazet @ 2019-06-20 15:21 UTC (permalink / raw)
To: Qian Cai; +Cc: David Miller, netdev, LKML
In-Reply-To: <1561042360-20480-1-git-send-email-cai@lca.pw>
On Thu, Jun 20, 2019 at 10:52 AM Qian Cai <cai@lca.pw> wrote:
>
> The linux-next commit "inet: fix various use-after-free in defrags
> units" [1] introduced compilation warnings,
>
> ./include/net/inet_frag.h:117:1: warning: 'inline' is not at beginning
> of declaration [-Wold-style-declaration]
> static void inline fqdir_pre_exit(struct fqdir *fqdir)
> ^~~~~~
Interesting warning, this is kind of new compiler major feature I guess :/
BTW :
$ git grep -n "static void inline" | wc -l
9
^ permalink raw reply
* Re: [PATCH net-next] netfilter: ipv6: Fix build error without CONFIG_IPV6
From: Yuehaibing @ 2019-06-20 15:26 UTC (permalink / raw)
To: pablo, kadlec, fw, davem, rdunlap
Cc: linux-kernel, coreteam, netfilter-devel, netdev
In-Reply-To: <20190612084715.21656-1-yuehaibing@huawei.com>
Friendly ping...
On 2019/6/12 16:47, YueHaibing wrote:
> If CONFIG_IPV6 is not set, building fails:
>
> net/bridge/netfilter/nf_conntrack_bridge.o: In function `nf_ct_bridge_pre':
> nf_conntrack_bridge.c:(.text+0x41c): undefined symbol `nf_ct_frag6_gather'
> net/bridge/netfilter/nf_conntrack_bridge.o: In function `nf_ct_bridge_post':
> nf_conntrack_bridge.c:(.text+0x820): undefined symbol `br_ip6_fragment'
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Fixes: c9bb6165a16e ("netfilter: nf_conntrack_bridge: fix CONFIG_IPV6=y")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> include/linux/netfilter_ipv6.h | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netfilter_ipv6.h b/include/linux/netfilter_ipv6.h
> index 3a3dc4b..0e1febc 100644
> --- a/include/linux/netfilter_ipv6.h
> +++ b/include/linux/netfilter_ipv6.h
> @@ -108,8 +108,11 @@ static inline int nf_ipv6_br_defrag(struct net *net, struct sk_buff *skb,
> return 1;
>
> return v6_ops->br_defrag(net, skb, user);
> -#else
> +#endif
> +#if IS_BUILTIN(CONFIG_IPV6)
> return nf_ct_frag6_gather(net, skb, user);
> +#else
> + return 1;
> #endif
> }
>
> @@ -133,8 +136,11 @@ static inline int nf_br_ip6_fragment(struct net *net, struct sock *sk,
> return 1;
>
> return v6_ops->br_fragment(net, sk, skb, data, output);
> -#else
> +#endif
> +#if IS_BUILTIN(CONFIG_IPV6)
> return br_ip6_fragment(net, sk, skb, data, output);
> +#else
> + return 1;
> #endif
> }
>
>
^ permalink raw reply
* Re: [PATCH v3 bpf-next 1/9] bpf: track spill/fill of constants
From: Alexei Starovoitov @ 2019-06-20 15:37 UTC (permalink / raw)
To: John Fastabend
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Network Development, bpf, Kernel Team
In-Reply-To: <5d0b13c990eaa_21bb2acd7a54c5b4a0@john-XPS-13-9370.notmuch>
On Wed, Jun 19, 2019 at 10:04 PM John Fastabend
<john.fastabend@gmail.com> wrote:
>
> working my way through the series now, but for this patch
>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
Thanks a lot for review!
It's landed now, but if you find anything I'll send
follow up patches.
Which I plan to do anyway for few things in backtracking logic.
^ permalink raw reply
* Re: [PATCH net] ipv6: fix neighbour resolution with raw socket
From: Nicolas Dichtel @ 2019-06-20 15:42 UTC (permalink / raw)
To: David Ahern, davem; +Cc: netdev
In-Reply-To: <fb3ed305-0161-8d6a-975c-54b29cfcb0ef@gmail.com>
Le 20/06/2019 à 17:12, David Ahern a écrit :
> On 6/20/19 6:34 AM, Nicolas Dichtel wrote:
>> The scenario is the following: the user uses a raw socket to send an ipv6
>> packet, destinated to a not-connected network, and specify a connected nh.
>> Here is the corresponding python script to reproduce this scenario:
>>
>> import socket
>> IPPROTO_RAW = 255
>> send_s = socket.socket(socket.AF_INET6, socket.SOCK_RAW, IPPROTO_RAW)
>> # scapy
>> # p = IPv6(src='fd00:100::1', dst='fd00:200::fa')/ICMPv6EchoRequest()
>> # str(p)
>> req = b'`\x00\x00\x00\x00\x08:@\xfd\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\xfd\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xfa\x80\x00\x81\xc0\x00\x00\x00\x00'
>> send_s.sendto(req, ('fd00:175::2', 0, 0, 0))
>>
>> fd00:175::/64 is a connected route and fd00:200::fa is not a connected
>> host.
>>
>> With this scenario, the kernel starts by sending a NS to resolve
>> fd00:175::2. When it receives the NA, it flushes its queue and try to send
>> the initial packet. But instead of sending it, it sends another NS to
>> resolve fd00:200::fa, which obvioulsy fails, thus the packet is dropped. If
>> the user sends again the packet, it now uses the right nh (fd00:175::2).
>>
>
> what's the local address and route setup? You reference fd00:100::1 and
> fd00:200::fa with connected route fd00:175::/64.
>
The test in done on the dut:
+-----+ +------+ +------+ +-----+
| tnl | | dut |.1 .2|router| | tnr |
| | | 2+-------------+2 | | |
| |.1 .2| |fd00:125::/64| |.2 .1| |
| 1+-------------+1 | | 1+-------------+1 |
| |fd00:100::/64| | | |fd00:200::/64| |
| | | |.1 .2| | | |
| | | 3+-------------+3 | | |
| | | |fd00:175::/64| | | |
+-----+ +------+ +------+ +-----+
On dut:
ip address add fd00:100::2/64 dev ntfp1
ip address add fd00:125::1/64 dev ntfp2
ip address add fd00:175::1/64 dev ntfp3
ip route add fd00:200::/64 via fd00:125::2
ip route add fd00:200::/120 nexthop via fd00:125::2 nexthop via fd00:175::2
Note that fd00:200::fa is not reachable but we expect to see the packet on the
host 'router'.
^ permalink raw reply
* Re: [linux-sunxi] [PATCH v7 5/6] drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
From: Jernej Škrabec @ 2019-06-20 15:47 UTC (permalink / raw)
To: linux-sunxi, megous
Cc: Maxime Ripard, Chen-Yu Tsai, Rob Herring, David Airlie,
Daniel Vetter, Mark Rutland, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, David S. Miller, Maxime Coquelin, dri-devel,
devicetree, linux-arm-kernel, linux-kernel, netdev, linux-stm32
In-Reply-To: <20190620134748.17866-6-megous@megous.com>
Dne četrtek, 20. junij 2019 ob 15:47:47 CEST je megous via linux-sunxi
napisal(a):
> From: Ondrej Jirman <megous@megous.com>
>
> Orange Pi 3 board requires enabling a voltage shifting circuit via GPIO
> for the DDC bus to be usable.
>
> Add support for hdmi-connector node's optional ddc-en-gpios property to
> support this use case.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
Best regards,
Jernej
^ permalink raw reply
* Re: [PATCH net-next] netfilter: ipv6: Fix build error without CONFIG_IPV6
From: Pablo Neira Ayuso @ 2019-06-20 15:50 UTC (permalink / raw)
To: Yuehaibing
Cc: kadlec, fw, davem, rdunlap, linux-kernel, coreteam,
netfilter-devel, netdev
In-Reply-To: <d2eba9e4-34be-f9bb-f0fd-024fe81d2b02@huawei.com>
On Thu, Jun 20, 2019 at 11:26:01PM +0800, Yuehaibing wrote:
> Friendly ping...
>
> On 2019/6/12 16:47, YueHaibing wrote:
> > If CONFIG_IPV6 is not set, building fails:
> >
> > net/bridge/netfilter/nf_conntrack_bridge.o: In function `nf_ct_bridge_pre':
> > nf_conntrack_bridge.c:(.text+0x41c): undefined symbol `nf_ct_frag6_gather'
> > net/bridge/netfilter/nf_conntrack_bridge.o: In function `nf_ct_bridge_post':
> > nf_conntrack_bridge.c:(.text+0x820): undefined symbol `br_ip6_fragment'
Is this one enough to fix this problem?
https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git/commit/?id=16e6427c88c5b7e7b6612f6c286d5f71d659e5be
Thanks.
> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Fixes: c9bb6165a16e ("netfilter: nf_conntrack_bridge: fix CONFIG_IPV6=y")
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > ---
> > include/linux/netfilter_ipv6.h | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/netfilter_ipv6.h b/include/linux/netfilter_ipv6.h
> > index 3a3dc4b..0e1febc 100644
> > --- a/include/linux/netfilter_ipv6.h
> > +++ b/include/linux/netfilter_ipv6.h
> > @@ -108,8 +108,11 @@ static inline int nf_ipv6_br_defrag(struct net *net, struct sk_buff *skb,
> > return 1;
> >
> > return v6_ops->br_defrag(net, skb, user);
> > -#else
> > +#endif
> > +#if IS_BUILTIN(CONFIG_IPV6)
> > return nf_ct_frag6_gather(net, skb, user);
> > +#else
> > + return 1;
> > #endif
> > }
> >
> > @@ -133,8 +136,11 @@ static inline int nf_br_ip6_fragment(struct net *net, struct sock *sk,
> > return 1;
> >
> > return v6_ops->br_fragment(net, sk, skb, data, output);
> > -#else
> > +#endif
> > +#if IS_BUILTIN(CONFIG_IPV6)
> > return br_ip6_fragment(net, sk, skb, data, output);
> > +#else
> > + return 1;
> > #endif
> > }
> >
> >
>
^ permalink raw reply
* Re: network unstable on odroid-c1/meson8b.
From: Heiner Kallweit @ 2019-06-20 15:53 UTC (permalink / raw)
To: Aymeric; +Cc: netdev, linux-amlogic, Martin Blumenstingl
In-Reply-To: <d2e298040f4887c547da11178f9ea64f@aplu.fr>
On 20.06.2019 09:55, Aymeric wrote:
> Hi,
> On 2019-06-20 00:14, Heiner Kallweit wrote:
>> On 19.06.2019 22:18, Aymeric wrote:
>>> Hello all,
>>>
>
>> Kernel 3.10 didn't have a dedicated RTL8211F PHY driver yet, therefore
>> I assume the genphy driver was used. Do you have a line with
>> "attached PHY driver" in dmesg output of the vendor kernel?
>
> No.
> Here is the full output of the dmesg from vendor kernel [¹].
>
> I've also noticed something strange, it might be linked, but mac address of the board is set to a random value when using mainline kernel and I've to set it manually but not when using vendor kernel.
>
>>
>> The dedicated PHY driver takes care of the tx delay, if the genphy
>> driver is used we have to rely on what uboot configured.
>> But if we indeed had an issue with a misconfigured delay, I think
>> the connection shouldn't be fine with just another link partner.
>> Just to have it tested you could make rtl8211f_config_init() in
>> drivers/net/phy/realtek.c a no-op (in current kernels).
>>
>
> I'm not an expert here, just adding a "return 0;" here[²] would be enough?
>
>> And you could compare at least the basic PHY registers 0x00 - 0x30
>> with both kernel versions, e.g. with phytool.
>>
>
> They are not the same but I don't know what I'm looking for, so for kernel 3.10 [³] and for kernel 5.1.12 [⁴].
>
> Aymeric
>
> [¹]: https://paste.aplu.fr/?38ef95b44ebdbfc3#G666/YbhgU+O+tdC/2HaimUCigm8ZTB44qvQip/HJ5A=
> [²]: https://github.com/torvalds/linux/blob/241e39004581475b2802cd63c111fec43bb0123e/drivers/net/phy/realtek.c#L164
> [³]: https://paste.aplu.fr/?2dde1c32d5c68f4c#6xIa8MjTm6jpI6citEJAqFTLMMHDjFZRet/M00/EwjU=
> [⁴]: https://paste.aplu.fr/?32130e9bcb05dde7#N/xdnvb5GklcJtiOxMpTCm+9gsUliRwH8X3dcwSV+ng=
>
The vendor kernel has some, but not really much magic:
https://github.com/hardkernel/linux/blob/odroidc-3.10.y/drivers/amlogic/ethernet/phy/am_rtl8211f.c
The write to RTL8211F_PHYCR2 is overwritten later, therefore we don't have to consider it.
The following should make the current Realtek PHY driver behave like in the vendor driver.
Could you test it?
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index a669945eb..f300b1cc9 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -163,6 +163,10 @@ static int rtl8211f_config_init(struct phy_device *phydev)
{
u16 val;
+ phy_write_paged(phydev, 0x0a43, 0x19, 0x0803);
+ genphy_soft_reset(phydev);
+ return 0;
+
/* enable TX-delay for rgmii-{id,txid}, and disable it for rgmii and
* rgmii-rxid. The RX-delay can be enabled by the external RXDLY pin.
*/
--
2.22.0
^ permalink raw reply related
* Re: [linux-sunxi] [PATCH v7 0/6] Add support for Orange Pi 3
From: Jernej Škrabec @ 2019-06-20 15:53 UTC (permalink / raw)
To: linux-sunxi, megous
Cc: Maxime Ripard, Chen-Yu Tsai, Rob Herring, David Airlie,
Daniel Vetter, Mark Rutland, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, David S. Miller, Maxime Coquelin, dri-devel,
devicetree, linux-arm-kernel, linux-kernel, netdev, linux-stm32
In-Reply-To: <20190620134748.17866-1-megous@megous.com>
Hi!
Dne četrtek, 20. junij 2019 ob 15:47:42 CEST je megous via linux-sunxi
napisal(a):
> From: Ondrej Jirman <megous@megous.com>
>
> This series implements support for Xunlong Orange Pi 3 board.
>
> - ethernet support (patches 1-3)
Correct me if I'm wrong, but patches 1-2 aren't strictly necessary for
OrangePi 3, right? H6 DTSI already has emac node with dual compatible (H6 and
A64) and since OrangePi 3 uses gigabit ethernet, quirk introduced by patches
1-2 are not needed.
However, it is nice to have this 100 Mbit fix, because most STB DTS will need
it.
Best regards,
Jernej
> - HDMI support (patches 4-6)
>
> For some people, ethernet doesn't work after reboot (but works on cold
> boot), when the stmmac driver is built into the kernel. It works when
> the driver is built as a module. It's either some timing issue, or power
> supply issue or a combination of both. Module build induces a power
> cycling of the phy.
>
> I encourage people with this issue, to build the driver into the kernel,
> and try to alter the reset timings for the phy in DTS or
> startup-delay-us and report the findings.
>
>
> Please take a look.
>
> thank you and regards,
> Ondrej Jirman
>
>
> Changes in v7:
> - dropped stored reference to connector_pdev as suggested by Jernej
> - added forgotten dt-bindings reviewed-by tag
>
> Changes in v6:
> - added dt-bindings reviewed-by tag
> - fix wording in stmmac commit (as suggested by Sergei)
>
> Changes in v5:
> - dropped already applied patches (pinctrl patches, mmc1 pinconf patch)
> - rename GMAC-3V3 -> GMAC-3V to match the schematic (Jagan)
> - changed hdmi-connector's ddc-supply property to ddc-en-gpios
> (Rob Herring)
>
> Changes in v4:
> - fix checkpatch warnings/style issues
> - use enum in struct sunxi_desc_function for io_bias_cfg_variant
> - collected acked-by's
> - fix compile error in drivers/pinctrl/sunxi/pinctrl-sun9i-a80-r.c:156
> caused by missing conversion from has_io_bias_cfg struct member
> (I've kept the acked-by, because it's a trivial change, but feel free
> to object.) (reported by Martin A. on github)
> I did not have A80 pinctrl enabled for some reason, so I did not catch
> this sooner.
> - dropped brcm firmware patch (was already applied)
> - dropped the wifi dts patch (will re-send after H6 RTC gets merged,
> along with bluetooth support, in a separate series)
>
> Changes in v3:
> - dropped already applied patches
> - changed pinctrl I/O bias selection constants to enum and renamed
> - added /omit-if-no-ref/ to mmc1_pins
> - made mmc1_pins default pinconf for mmc1 in H6 dtsi
> - move ddc-supply to HDMI connector node, updated patch descriptions,
> changed dt-bindings docs
>
> Changes in v2:
> - added dt-bindings documentation for the board's compatible string
> (suggested by Clement)
> - addressed checkpatch warnings and code formatting issues (on Maxime's
> suggestions)
> - stmmac: dropped useless parenthesis, reworded description of the patch
> (suggested by Sergei)
> - drop useles dev_info() about the selected io bias voltage
> - docummented io voltage bias selection variant macros
> - wifi: marked WiFi DTS patch and realted mmc1_pins as "DO NOT MERGE",
> because wifi depends on H6 RTC support that's not merged yet (suggested
> by Clement)
> - added missing signed-of-bys
> - changed &usb2otg dr_mode to otg, and added a note about VBUS
> - improved wording of HDMI driver's DDC power supply patch
>
> Icenowy Zheng (2):
> net: stmmac: sun8i: add support for Allwinner H6 EMAC
> net: stmmac: sun8i: force select external PHY when no internal one
>
> Ondrej Jirman (4):
> arm64: dts: allwinner: orange-pi-3: Enable ethernet
> dt-bindings: display: hdmi-connector: Support DDC bus enable
> drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
> arm64: dts: allwinner: orange-pi-3: Enable HDMI output
>
> .../display/connector/hdmi-connector.txt | 1 +
> .../dts/allwinner/sun50i-h6-orangepi-3.dts | 70 +++++++++++++++++++
> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 54 ++++++++++++--
> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 +
> .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 21 ++++++
> 5 files changed, 144 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH] mm: mempolicy: handle vma with unmovable pages mapped correctly in mbind
From: Yang Shi @ 2019-06-20 16:08 UTC (permalink / raw)
To: Vlastimil Babka, Michal Hocko
Cc: akpm, mgorman, linux-mm, linux-kernel, Eric Dumazet,
David S. Miller, netdev
In-Reply-To: <d81b36bb-876e-917a-6115-cedf496b4923@suse.cz>
On 6/20/19 12:18 AM, Vlastimil Babka wrote:
> On 6/19/19 8:19 PM, Yang Shi wrote:
>>>>> This is getting even more muddy TBH. Is there any reason that we
>>>>> have to
>>>>> handle this problem during the isolation phase rather the migration?
>>>> I think it was already said that if pages can't be isolated, then
>>>> migration phase won't process them, so they're just ignored.
>>> Yes，exactly.
>>>
>>>> However I think the patch is wrong to abort immediately when
>>>> encountering such page that cannot be isolated (AFAICS). IMHO it should
>>>> still try to migrate everything it can, and only then return -EIO.
>>> It is fine too. I don't see mbind semantics define how to handle such
>>> case other than returning -EIO.
> I think it does. There's:
> If MPOL_MF_MOVE is specified in flags, then the kernel *will attempt to
> move all the existing pages* ... If MPOL_MF_STRICT is also specified,
> then the call fails with the error *EIO if some pages could not be moved*
>
> Aborting immediately would be against the attempt to move all.
>
>> By looking into the code, it looks not that easy as what I thought.
>> do_mbind() would check the return value of queue_pages_range(), it just
>> applies the policy and manipulates vmas as long as the return value is 0
>> (success), then migrate pages on the list. We could put the movable
>> pages on the list by not breaking immediately, but they will be ignored.
>> If we migrate the pages regardless of the return value, it may break the
>> policy since the policy will *not* be applied at all.
> I think we just need to remember if there was at least one page that
> failed isolation or migration, but keep working, and in the end return
> EIO if there was such page(s). I don't think it breaks the policy. Once
> pages are allocated in a mapping, changing the policy is a best effort
> thing anyway.
The current behavior is:
If queue_pages_range() return -EIO (vma is not migratable, ignore other
conditions since we just focus on page migration), the policy won't be
set and no page will be migrated.
However, the problem here is the vma might look migratable, but some or
all the underlying pages are unmovable. So, my patch assumes the vma is
*not* migratable if at least one page is unmovable. I'm not sure if it
is possible to have both movable and unmovable pages for the same
mapping or not, I'm supposed the vma would be split much earlier.
If we don't abort immediately, then we record if there is unmovable
page, then we could do:
#1. Still follows the current behavior (then why not abort immediately?)
#2. Set mempolicy then migrate all the migratable pages. But, we may end
up with the pages on node A, but the policy says node B. Doesn't it
break the policy?
>
>>>
^ permalink raw reply
* Re: [PATCH net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Neil Horman @ 2019-06-20 16:14 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <CAF=yD-KFZBS7PpvvBkHS5jQdjRr4tWpeHmb7=9QPmvD-RTcpYw@mail.gmail.com>
On Thu, Jun 20, 2019 at 11:16:13AM -0400, Willem de Bruijn wrote:
> On Thu, Jun 20, 2019 at 10:24 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Thu, Jun 20, 2019 at 09:41:30AM -0400, Willem de Bruijn wrote:
> > > On Wed, Jun 19, 2019 at 4:26 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >
> > > > When an application is run that:
> > > > a) Sets its scheduler to be SCHED_FIFO
> > > > and
> > > > b) Opens a memory mapped AF_PACKET socket, and sends frames with the
> > > > MSG_DONTWAIT flag cleared, its possible for the application to hang
> > > > forever in the kernel. This occurs because when waiting, the code in
> > > > tpacket_snd calls schedule, which under normal circumstances allows
> > > > other tasks to run, including ksoftirqd, which in some cases is
> > > > responsible for freeing the transmitted skb (which in AF_PACKET calls a
> > > > destructor that flips the status bit of the transmitted frame back to
> > > > available, allowing the transmitting task to complete).
> > > >
> > > > However, when the calling application is SCHED_FIFO, its priority is
> > > > such that the schedule call immediately places the task back on the cpu,
> > > > preventing ksoftirqd from freeing the skb, which in turn prevents the
> > > > transmitting task from detecting that the transmission is complete.
> > > >
> > > > We can fix this by converting the schedule call to a completion
> > > > mechanism. By using a completion queue, we force the calling task, when
> > > > it detects there are no more frames to send, to schedule itself off the
> > > > cpu until such time as the last transmitted skb is freed, allowing
> > > > forward progress to be made.
> > > >
> > > > Tested by myself and the reporter, with good results
> > > >
> > > > Appies to the net tree
> > > >
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > Reported-by: Matteo Croce <mcroce@redhat.com>
> > > > CC: "David S. Miller" <davem@davemloft.net>
> > > > ---
> > >
> > > This is a complex change for a narrow configuration. Isn't a
> > > SCHED_FIFO process preempting ksoftirqd a potential problem for other
> > > networking workloads as well? And the right configuration to always
> > > increase ksoftirqd priority when increasing another process's
> > > priority? Also, even when ksoftirqd kicks in, isn't some progress
> > > still made on the local_bh_enable reached from schedule()?
> > >
> >
> > A few questions here to answer:
>
> Thanks for the detailed explanation.
>
Gladly.
> > Regarding other protocols having this problem, thats not the case, because non
> > packet sockets honor the SK_SNDTIMEO option here (i.e. they sleep for a period
> > of time specified by the SNDTIMEO option if MSG_DONTWAIT isn't set. We could
> > certainly do that, but the current implementation doesn't (opting instead to
> > wait indefinately until the respective packet(s) have transmitted or errored
> > out), and I wanted to maintain that behavior. If there is consensus that packet
> > sockets should honor SNDTIMEO, then I can certainly do that.
> >
> > As for progress made by calling local_bh_enable, My read of the code doesn't
> > have the scheduler calling local_bh_enable at all. Instead schedule uses
> > preempt_disable/preempt_enable_no_resched() to gain exlcusive access to the cpu,
> > which ignores pending softirqs on re-enablement.
>
> Ah, I'm mistaken there, then.
>
> > Perhaps that needs to change,
> > but I'm averse to making scheduler changes for this (the aforementioned concern
> > about complex changes for a narrow use case)
> >
> > Regarding raising the priority of ksoftirqd, that could be a solution, but the
> > priority would need to be raised to a high priority SCHED_FIFO parameter, and
> > that gets back to making complex changes for a narrow problem domain
> >
> > As for the comlexity of the of the solution, I think this is, given your
> > comments the least complex and intrusive change to solve the given problem.
>
> Could it be simpler to ensure do_softirq() gets run here? That would
> allow progress for this case.
>
I'm not sure. On the surface, we certainly could do it, but inserting a call to
do_softirq, either directly, or indirectly through some other mechanism seems
like a non-obvious fix, and may lead to confusion down the road. I'm hesitant
to pursue such a soultion without some evidence it would make a better solution.
> > We
> > need to find a way to force the calling task off the cpu while the asynchronous
> > operations in the transmit path complete, and we can do that this way, or by
> > honoring SK_SNDTIMEO. I'm fine with doing the latter, but I didn't want to
> > alter the current protocol behavior without consensus on that.
>
> In general SCHED_FIFO is dangerous with regard to stalling other
> progress, incl. ksoftirqd. But it does appear that this packet socket
> case is special inside networking in calling schedule() directly here.
>
> If converting that, should it convert to logic more akin to other
> sockets, like sock_wait_for_wmem? I haven't had a chance to read up on
> the pros and cons of completion here yet, sorry. Didn't want to delay
> responding until after I get a chance.
>
That would be the solution described above (i.e. honoring SK_SNDTIMEO.
Basically you call sock_send_waittimeo, which returns a timeout value, or 0 if
MSG_DONTWAIT is set), then you block for that period of time waiting for
transmit completion. I'm happy to implement that solution, but I'd like to get
some clarity as to if there is a reason we don't currently honor that socket
option now before I change the behavior that way.
Dave, do you have any insight into AF_PACKET history as to why we would ignore
the send timeout socket option here?
Best
Neil
^ permalink raw reply
* Re: [PATCH net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Willem de Bruijn @ 2019-06-20 16:18 UTC (permalink / raw)
To: Neil Horman; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <20190620161411.GE18890@hmswarspite.think-freely.org>
On Thu, Jun 20, 2019 at 12:14 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Thu, Jun 20, 2019 at 11:16:13AM -0400, Willem de Bruijn wrote:
> > On Thu, Jun 20, 2019 at 10:24 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Thu, Jun 20, 2019 at 09:41:30AM -0400, Willem de Bruijn wrote:
> > > > On Wed, Jun 19, 2019 at 4:26 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > >
> > > > > When an application is run that:
> > > > > a) Sets its scheduler to be SCHED_FIFO
> > > > > and
> > > > > b) Opens a memory mapped AF_PACKET socket, and sends frames with the
> > > > > MSG_DONTWAIT flag cleared, its possible for the application to hang
> > > > > forever in the kernel. This occurs because when waiting, the code in
> > > > > tpacket_snd calls schedule, which under normal circumstances allows
> > > > > other tasks to run, including ksoftirqd, which in some cases is
> > > > > responsible for freeing the transmitted skb (which in AF_PACKET calls a
> > > > > destructor that flips the status bit of the transmitted frame back to
> > > > > available, allowing the transmitting task to complete).
> > > > >
> > > > > However, when the calling application is SCHED_FIFO, its priority is
> > > > > such that the schedule call immediately places the task back on the cpu,
> > > > > preventing ksoftirqd from freeing the skb, which in turn prevents the
> > > > > transmitting task from detecting that the transmission is complete.
> > > > >
> > > > > We can fix this by converting the schedule call to a completion
> > > > > mechanism. By using a completion queue, we force the calling task, when
> > > > > it detects there are no more frames to send, to schedule itself off the
> > > > > cpu until such time as the last transmitted skb is freed, allowing
> > > > > forward progress to be made.
> > > > >
> > > > > Tested by myself and the reporter, with good results
> > > > >
> > > > > Appies to the net tree
> > > > >
> > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > Reported-by: Matteo Croce <mcroce@redhat.com>
> > > > > CC: "David S. Miller" <davem@davemloft.net>
> > > > > ---
> > > >
> > > > This is a complex change for a narrow configuration. Isn't a
> > > > SCHED_FIFO process preempting ksoftirqd a potential problem for other
> > > > networking workloads as well? And the right configuration to always
> > > > increase ksoftirqd priority when increasing another process's
> > > > priority? Also, even when ksoftirqd kicks in, isn't some progress
> > > > still made on the local_bh_enable reached from schedule()?
> > > >
> > >
> > > A few questions here to answer:
> >
> > Thanks for the detailed explanation.
> >
> Gladly.
>
> > > Regarding other protocols having this problem, thats not the case, because non
> > > packet sockets honor the SK_SNDTIMEO option here (i.e. they sleep for a period
> > > of time specified by the SNDTIMEO option if MSG_DONTWAIT isn't set. We could
> > > certainly do that, but the current implementation doesn't (opting instead to
> > > wait indefinately until the respective packet(s) have transmitted or errored
> > > out), and I wanted to maintain that behavior. If there is consensus that packet
> > > sockets should honor SNDTIMEO, then I can certainly do that.
> > >
> > > As for progress made by calling local_bh_enable, My read of the code doesn't
> > > have the scheduler calling local_bh_enable at all. Instead schedule uses
> > > preempt_disable/preempt_enable_no_resched() to gain exlcusive access to the cpu,
> > > which ignores pending softirqs on re-enablement.
> >
> > Ah, I'm mistaken there, then.
> >
> > > Perhaps that needs to change,
> > > but I'm averse to making scheduler changes for this (the aforementioned concern
> > > about complex changes for a narrow use case)
> > >
> > > Regarding raising the priority of ksoftirqd, that could be a solution, but the
> > > priority would need to be raised to a high priority SCHED_FIFO parameter, and
> > > that gets back to making complex changes for a narrow problem domain
> > >
> > > As for the comlexity of the of the solution, I think this is, given your
> > > comments the least complex and intrusive change to solve the given problem.
> >
> > Could it be simpler to ensure do_softirq() gets run here? That would
> > allow progress for this case.
> >
> I'm not sure. On the surface, we certainly could do it, but inserting a call to
> do_softirq, either directly, or indirectly through some other mechanism seems
> like a non-obvious fix, and may lead to confusion down the road. I'm hesitant
> to pursue such a soultion without some evidence it would make a better solution.
>
> > > We
> > > need to find a way to force the calling task off the cpu while the asynchronous
> > > operations in the transmit path complete, and we can do that this way, or by
> > > honoring SK_SNDTIMEO. I'm fine with doing the latter, but I didn't want to
> > > alter the current protocol behavior without consensus on that.
> >
> > In general SCHED_FIFO is dangerous with regard to stalling other
> > progress, incl. ksoftirqd. But it does appear that this packet socket
> > case is special inside networking in calling schedule() directly here.
> >
> > If converting that, should it convert to logic more akin to other
> > sockets, like sock_wait_for_wmem? I haven't had a chance to read up on
> > the pros and cons of completion here yet, sorry. Didn't want to delay
> > responding until after I get a chance.
> >
> That would be the solution described above (i.e. honoring SK_SNDTIMEO.
> Basically you call sock_send_waittimeo, which returns a timeout value, or 0 if
> MSG_DONTWAIT is set), then you block for that period of time waiting for
> transmit completion.
From an ABI point of view, starting to support SK_SNDTIMEO where it
currently is not implemented certainly seems fine.
> I'm happy to implement that solution, but I'd like to get
> some clarity as to if there is a reason we don't currently honor that socket
> option now before I change the behavior that way.
>
> Dave, do you have any insight into AF_PACKET history as to why we would ignore
> the send timeout socket option here?
On the point of calling schedule(): even if that is rare, there are a lot of
other cond_resched() calls that may have the same starvation issue
with SCHED_FIFO and ksoftirqd.
^ permalink raw reply
* [PATCH nf-next v5] netfilter: add support for matching IPv4 options
From: Stephen Suryaputra @ 2019-06-20 16:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Stephen Suryaputra
This is the kernel change for the overall changes with this description:
Add capability to have rules matching IPv4 options. This is developed
mainly to support dropping of IP packets with loose and/or strict source
route route options.
v2: Fix style issues. Make this work with NFPROTO_INET (inet tables),
NFPROTO_BRIDGE and the NFPROTO_NETDEV families. Check skb->protocol.
Remove ability to input IP header offset for ipv4_find_option()
function (all per Pablo Neira Ayuso).
v3: Remove unused ipv4_find_option() arguments (per Pablo Neira Ayuso).
v4: Change the code to not follow ipv6_find_hdr() and just do what are
needed to support source-route, record route and router alert (per
Pablo Neira Ayuso). Fix bugs that are introduced while addressing
review comments.
v5: Add nft_exthdr_ipv4_init(). Remove redundant check of IP version
(feedback from Pablo Neira Ayuso).
Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
include/uapi/linux/netfilter/nf_tables.h | 2 +
net/ipv4/ip_options.c | 1 +
net/netfilter/nft_exthdr.c | 133 +++++++++++++++++++++++
3 files changed, 136 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 505393c6e959..168d741f42c5 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -730,10 +730,12 @@ enum nft_exthdr_flags {
*
* @NFT_EXTHDR_OP_IPV6: match against ipv6 extension headers
* @NFT_EXTHDR_OP_TCP: match against tcp options
+ * @NFT_EXTHDR_OP_IPV4: match against ipv4 options
*/
enum nft_exthdr_op {
NFT_EXTHDR_OP_IPV6,
NFT_EXTHDR_OP_TCPOPT,
+ NFT_EXTHDR_OP_IPV4,
__NFT_EXTHDR_OP_MAX
};
#define NFT_EXTHDR_OP_MAX (__NFT_EXTHDR_OP_MAX - 1)
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index 3db31bb9df50..ddaa01ec2bce 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -473,6 +473,7 @@ int __ip_options_compile(struct net *net,
*info = htonl((pp_ptr-iph)<<24);
return -EINVAL;
}
+EXPORT_SYMBOL(__ip_options_compile);
int ip_options_compile(struct net *net,
struct ip_options *opt, struct sk_buff *skb)
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 45c8a6c07783..8032b2937c7f 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -62,6 +62,103 @@ static void nft_exthdr_ipv6_eval(const struct nft_expr *expr,
regs->verdict.code = NFT_BREAK;
}
+/* find the offset to specified option.
+ *
+ * If target header is found, its offset is set in *offset and return option
+ * number. Otherwise, return negative error.
+ *
+ * If the first fragment doesn't contain the End of Options it is considered
+ * invalid.
+ */
+static int ipv4_find_option(struct net *net, struct sk_buff *skb,
+ unsigned int *offset, int target)
+{
+ unsigned char optbuf[sizeof(struct ip_options) + 40];
+ struct ip_options *opt = (struct ip_options *)optbuf;
+ struct iphdr *iph, _iph;
+ unsigned int start;
+ bool found = false;
+ __be32 info;
+ int optlen;
+
+ iph = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
+ if (!iph)
+ return -EBADMSG;
+ start = sizeof(struct iphdr);
+
+ optlen = iph->ihl * 4 - (int)sizeof(struct iphdr);
+ if (optlen <= 0)
+ return -ENOENT;
+
+ memset(opt, 0, sizeof(struct ip_options));
+ /* Copy the options since __ip_options_compile() modifies
+ * the options.
+ */
+ if (skb_copy_bits(skb, start, opt->__data, optlen))
+ return -EBADMSG;
+ opt->optlen = optlen;
+
+ if (__ip_options_compile(net, opt, NULL, &info))
+ return -EBADMSG;
+
+ switch (target) {
+ case IPOPT_SSRR:
+ case IPOPT_LSRR:
+ if (!opt->srr)
+ break;
+ found = target == IPOPT_SSRR ? opt->is_strictroute :
+ !opt->is_strictroute;
+ if (found)
+ *offset = opt->srr + start;
+ break;
+ case IPOPT_RR:
+ if (!opt->rr)
+ break;
+ *offset = opt->rr + start;
+ found = true;
+ break;
+ case IPOPT_RA:
+ if (!opt->router_alert)
+ break;
+ *offset = opt->router_alert + start;
+ found = true;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+ return found ? target : -ENOENT;
+}
+
+static void nft_exthdr_ipv4_eval(const struct nft_expr *expr,
+ struct nft_regs *regs,
+ const struct nft_pktinfo *pkt)
+{
+ struct nft_exthdr *priv = nft_expr_priv(expr);
+ u32 *dest = ®s->data[priv->dreg];
+ struct sk_buff *skb = pkt->skb;
+ unsigned int offset;
+ int err;
+
+ if (skb->protocol != htons(ETH_P_IP))
+ goto err;
+
+ err = ipv4_find_option(nft_net(pkt), skb, &offset, priv->type);
+ if (priv->flags & NFT_EXTHDR_F_PRESENT) {
+ *dest = (err >= 0);
+ return;
+ } else if (err < 0) {
+ goto err;
+ }
+ offset += priv->offset;
+
+ dest[priv->len / NFT_REG32_SIZE] = 0;
+ if (skb_copy_bits(pkt->skb, offset, dest, priv->len) < 0)
+ goto err;
+ return;
+err:
+ regs->verdict.code = NFT_BREAK;
+}
+
static void *
nft_tcp_header_pointer(const struct nft_pktinfo *pkt,
unsigned int len, void *buffer, unsigned int *tcphdr_len)
@@ -315,6 +412,28 @@ static int nft_exthdr_tcp_set_init(const struct nft_ctx *ctx,
return nft_validate_register_load(priv->sreg, priv->len);
}
+static int nft_exthdr_ipv4_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
+{
+ struct nft_exthdr *priv = nft_expr_priv(expr);
+ int err = nft_exthdr_init(ctx, expr, tb);
+
+ if (err < 0)
+ return err;
+
+ switch (priv->type) {
+ case IPOPT_SSRR:
+ case IPOPT_LSRR:
+ case IPOPT_RR:
+ case IPOPT_RA:
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
+
static int nft_exthdr_dump_common(struct sk_buff *skb, const struct nft_exthdr *priv)
{
if (nla_put_u8(skb, NFTA_EXTHDR_TYPE, priv->type))
@@ -361,6 +480,14 @@ static const struct nft_expr_ops nft_exthdr_ipv6_ops = {
.dump = nft_exthdr_dump,
};
+static const struct nft_expr_ops nft_exthdr_ipv4_ops = {
+ .type = &nft_exthdr_type,
+ .size = NFT_EXPR_SIZE(sizeof(struct nft_exthdr)),
+ .eval = nft_exthdr_ipv4_eval,
+ .init = nft_exthdr_ipv4_init,
+ .dump = nft_exthdr_dump,
+};
+
static const struct nft_expr_ops nft_exthdr_tcp_ops = {
.type = &nft_exthdr_type,
.size = NFT_EXPR_SIZE(sizeof(struct nft_exthdr)),
@@ -401,6 +528,12 @@ nft_exthdr_select_ops(const struct nft_ctx *ctx,
if (tb[NFTA_EXTHDR_DREG])
return &nft_exthdr_ipv6_ops;
break;
+ case NFT_EXTHDR_OP_IPV4:
+ if (ctx->family != NFPROTO_IPV6) {
+ if (tb[NFTA_EXTHDR_DREG])
+ return &nft_exthdr_ipv4_ops;
+ }
+ break;
}
return ERR_PTR(-EOPNOTSUPP);
--
2.17.1
^ permalink raw reply related
* Re: [linux-sunxi] [PATCH v7 0/6] Add support for Orange Pi 3
From: Ondřej Jirman @ 2019-06-20 16:34 UTC (permalink / raw)
To: Jernej Škrabec
Cc: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
David Airlie, Daniel Vetter, Mark Rutland, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin,
dri-devel, devicetree, linux-arm-kernel, linux-kernel, netdev,
linux-stm32
In-Reply-To: <2263144.KN5DhQ2VKD@jernej-laptop>
Hi Jernej,
On Thu, Jun 20, 2019 at 05:53:58PM +0200, Jernej Škrabec wrote:
> Hi!
>
> Dne četrtek, 20. junij 2019 ob 15:47:42 CEST je megous via linux-sunxi
> napisal(a):
> > From: Ondrej Jirman <megous@megous.com>
> >
> > This series implements support for Xunlong Orange Pi 3 board.
> >
> > - ethernet support (patches 1-3)
>
> Correct me if I'm wrong, but patches 1-2 aren't strictly necessary for
> OrangePi 3, right? H6 DTSI already has emac node with dual compatible (H6 and
> A64) and since OrangePi 3 uses gigabit ethernet, quirk introduced by patches
> 1-2 are not needed.
I've checked with u-boot and md.l 0x03000030 (syscon_field) and the actual
default value there on cold boot is 0x58000, just like on H3.
H3_EPHY_SELECT is BIT(15)
That means that those patches (1 and 2) are both doing the same thing, basicaly.
H3_EPHY_SELECT bit needs to be cleared, and it is cleared either explicitly, or
via default_syscon_value = 0x50000. It's also cleared incidentally by using
emac_variant_a64, because it has default_syscon_value set to 0.
Meaning of those remaining set bits on H6[1] are the same as on H3. Bit 16 is
SHUTDOWN (on 1) and bit 18 is CLK_SEL. At least SHUTDOWN bit should be kept
high, as it keeps the EPHY shut down. Normally that would be ensured by the
code, but only if soc_has_internal_phy is true, which it is not for
emac_variant_a64.
Thus the patch adds the emac_variant_h6 with a different default_syscon_value
from A64.
Dose the SHUTDOWN bit matter on H6? I don't know. I'm just trying to keep the
default values of these bits unchanged. Maybe it would be nicer to have
default_syscon_value be 0x58000 on H6, to avoid the boot warning.
dwmac-sun8i 5020000.ethernet: Current syscon value is not the default 58000 (expect 50000)
The same warning is there with A64 compatible (with "expect 0").
[1] See page 238 in H6 manual.
regards,
o.
> However, it is nice to have this 100 Mbit fix, because most STB DTS will need
> it.
>
> Best regards,
> Jernej
>
> > - HDMI support (patches 4-6)
> >
> > For some people, ethernet doesn't work after reboot (but works on cold
> > boot), when the stmmac driver is built into the kernel. It works when
> > the driver is built as a module. It's either some timing issue, or power
> > supply issue or a combination of both. Module build induces a power
> > cycling of the phy.
> >
> > I encourage people with this issue, to build the driver into the kernel,
> > and try to alter the reset timings for the phy in DTS or
> > startup-delay-us and report the findings.
> >
> >
> > Please take a look.
> >
> > thank you and regards,
> > Ondrej Jirman
> >
> >
> > Changes in v7:
> > - dropped stored reference to connector_pdev as suggested by Jernej
> > - added forgotten dt-bindings reviewed-by tag
> >
> > Changes in v6:
> > - added dt-bindings reviewed-by tag
> > - fix wording in stmmac commit (as suggested by Sergei)
> >
> > Changes in v5:
> > - dropped already applied patches (pinctrl patches, mmc1 pinconf patch)
> > - rename GMAC-3V3 -> GMAC-3V to match the schematic (Jagan)
> > - changed hdmi-connector's ddc-supply property to ddc-en-gpios
> > (Rob Herring)
> >
> > Changes in v4:
> > - fix checkpatch warnings/style issues
> > - use enum in struct sunxi_desc_function for io_bias_cfg_variant
> > - collected acked-by's
> > - fix compile error in drivers/pinctrl/sunxi/pinctrl-sun9i-a80-r.c:156
> > caused by missing conversion from has_io_bias_cfg struct member
> > (I've kept the acked-by, because it's a trivial change, but feel free
> > to object.) (reported by Martin A. on github)
> > I did not have A80 pinctrl enabled for some reason, so I did not catch
> > this sooner.
> > - dropped brcm firmware patch (was already applied)
> > - dropped the wifi dts patch (will re-send after H6 RTC gets merged,
> > along with bluetooth support, in a separate series)
> >
> > Changes in v3:
> > - dropped already applied patches
> > - changed pinctrl I/O bias selection constants to enum and renamed
> > - added /omit-if-no-ref/ to mmc1_pins
> > - made mmc1_pins default pinconf for mmc1 in H6 dtsi
> > - move ddc-supply to HDMI connector node, updated patch descriptions,
> > changed dt-bindings docs
> >
> > Changes in v2:
> > - added dt-bindings documentation for the board's compatible string
> > (suggested by Clement)
> > - addressed checkpatch warnings and code formatting issues (on Maxime's
> > suggestions)
> > - stmmac: dropped useless parenthesis, reworded description of the patch
> > (suggested by Sergei)
> > - drop useles dev_info() about the selected io bias voltage
> > - docummented io voltage bias selection variant macros
> > - wifi: marked WiFi DTS patch and realted mmc1_pins as "DO NOT MERGE",
> > because wifi depends on H6 RTC support that's not merged yet (suggested
> > by Clement)
> > - added missing signed-of-bys
> > - changed &usb2otg dr_mode to otg, and added a note about VBUS
> > - improved wording of HDMI driver's DDC power supply patch
> >
> > Icenowy Zheng (2):
> > net: stmmac: sun8i: add support for Allwinner H6 EMAC
> > net: stmmac: sun8i: force select external PHY when no internal one
> >
> > Ondrej Jirman (4):
> > arm64: dts: allwinner: orange-pi-3: Enable ethernet
> > dt-bindings: display: hdmi-connector: Support DDC bus enable
> > drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
> > arm64: dts: allwinner: orange-pi-3: Enable HDMI output
> >
> > .../display/connector/hdmi-connector.txt | 1 +
> > .../dts/allwinner/sun50i-h6-orangepi-3.dts | 70 +++++++++++++++++++
> > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 54 ++++++++++++--
> > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 +
> > .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 21 ++++++
> > 5 files changed, 144 insertions(+), 4 deletions(-)
>
>
>
>
^ permalink raw reply
* Re: [PATCH net] ipv6: fix neighbour resolution with raw socket
From: David Ahern @ 2019-06-20 16:36 UTC (permalink / raw)
To: nicolas.dichtel, davem; +Cc: netdev
In-Reply-To: <3066f846-f549-f982-7bc0-1f9bc3d87b94@6wind.com>
On 6/20/19 9:42 AM, Nicolas Dichtel wrote:
> Le 20/06/2019 à 17:12, David Ahern a écrit :
>> On 6/20/19 6:34 AM, Nicolas Dichtel wrote:
>>> The scenario is the following: the user uses a raw socket to send an ipv6
>>> packet, destinated to a not-connected network, and specify a connected nh.
>>> Here is the corresponding python script to reproduce this scenario:
>>>
>>> import socket
>>> IPPROTO_RAW = 255
>>> send_s = socket.socket(socket.AF_INET6, socket.SOCK_RAW, IPPROTO_RAW)
>>> # scapy
>>> # p = IPv6(src='fd00:100::1', dst='fd00:200::fa')/ICMPv6EchoRequest()
>>> # str(p)
>>> req = b'`\x00\x00\x00\x00\x08:@\xfd\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\xfd\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xfa\x80\x00\x81\xc0\x00\x00\x00\x00'
>>> send_s.sendto(req, ('fd00:175::2', 0, 0, 0))
>>>
>>> fd00:175::/64 is a connected route and fd00:200::fa is not a connected
>>> host.
>>>
>>> With this scenario, the kernel starts by sending a NS to resolve
>>> fd00:175::2. When it receives the NA, it flushes its queue and try to send
>>> the initial packet. But instead of sending it, it sends another NS to
>>> resolve fd00:200::fa, which obvioulsy fails, thus the packet is dropped. If
>>> the user sends again the packet, it now uses the right nh (fd00:175::2).
>>>
>>
>> what's the local address and route setup? You reference fd00:100::1 and
>> fd00:200::fa with connected route fd00:175::/64.
>>
>
> The test in done on the dut:
>
> +-----+ +------+ +------+ +-----+
> | tnl | | dut |.1 .2|router| | tnr |
> | | | 2+-------------+2 | | |
> | |.1 .2| |fd00:125::/64| |.2 .1| |
> | 1+-------------+1 | | 1+-------------+1 |
> | |fd00:100::/64| | | |fd00:200::/64| |
> | | | |.1 .2| | | |
> | | | 3+-------------+3 | | |
> | | | |fd00:175::/64| | | |
> +-----+ +------+ +------+ +-----+
>
> On dut:
> ip address add fd00:100::2/64 dev ntfp1
> ip address add fd00:125::1/64 dev ntfp2
> ip address add fd00:175::1/64 dev ntfp3
> ip route add fd00:200::/64 via fd00:125::2
> ip route add fd00:200::/120 nexthop via fd00:125::2 nexthop via fd00:175::2
>
> Note that fd00:200::fa is not reachable but we expect to see the packet on the
> host 'router'.
>
gotcha. Thanks for the diagram.
Reviewed-by: David Ahern <dsahern@gmail.com>
You don't have a fixes tag, but this should go to stable releases.
Also, this does not fix the forwarding case. For the forwarding case I
still see it trying to resolve fd00:200::fa from dut.
Namespace version of the above setup:
create_ns() {
local ns=$1
ip netns add ${ns}
ip -netns ${ns} link set lo up
ip netns exec ${ns} sysctl -qw net.ipv4.ip_forward=1
ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.keep_addr_on_down=1
ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.forwarding=1
ip netns exec ${ns} sysctl -qw net.ipv6.conf.default.forwarding=1
}
create_ns tnl
create_ns dut
create_ns router
create_ns tnr
ip -netns tnl li add eth1 type veth peer name eth1d
ip -netns tnl li set eth1d netns dut name eth1
ip -netns tnl li set eth1 up
ip -netns tnl addr add fd00:100::1/64 dev eth1
ip -netns tnl -6 ro add default via fd00:100::2
ip -netns dut li set eth1 up
ip -netns dut addr add fd00:100::2/64 dev eth1
ip -netns dut li add eth2 type veth peer name eth2r
ip -netns dut li set eth2r netns router name eth2
ip -netns dut li set eth2 up
ip -netns dut addr add dev eth2 fd00:125::1/64
ip -netns router li set eth2 up
ip -netns router addr add dev eth2 fd00:125::2/64
ip -netns dut li add eth3 type veth peer name eth3r
ip -netns dut li set eth3r netns router name eth3
ip -netns dut li set eth3 up
ip -netns dut addr add dev eth3 fd00:175::1/64
ip -netns router li set eth3 up
ip -netns router addr add dev eth3 fd00:175::2/64
ip -netns router li add eth1 type veth peer name eth1t
ip -netns router li set eth1t netns tnr name eth1
ip -netns router li set eth1 up
ip -netns router addr add dev eth1 fd00:200::2/64
ip -netns tnr li set eth1 up
ip -netns tnr addr add dev eth1 fd00:200::1/64
ip -netns dut route add fd00:200::/64 via fd00:125::2
ip -netns dut route add fd00:200::/120 nexthop via fd00:125::2 nexthop
via fd00:175::2
^ permalink raw reply
* Re: [PATCH net] ipv6: fix neighbour resolution with raw socket
From: David Ahern @ 2019-06-20 16:47 UTC (permalink / raw)
To: nicolas.dichtel, davem; +Cc: netdev
In-Reply-To: <c1e3d444-a7c9-def4-9f16-37db5dd071fe@gmail.com>
On 6/20/19 10:36 AM, David Ahern wrote:
>
> Also, this does not fix the forwarding case. For the forwarding case I
> still see it trying to resolve fd00:200::fa from dut.
nevermind. I used the wrong address for the forwarding case.
^ permalink raw reply
* Re: [PATCH net-next v4 1/7] igb: clear out tstamp after sending the packet
From: Patel, Vedang @ 2019-06-20 16:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, Kirsher, Jeffrey T, David Miller,
Jamal Hadi Salim, Cong Wang, Jiri Pirko,
intel-wired-lan@lists.osuosl.org, Gomes, Vinicius, l@dorileo.org,
Jakub Kicinski, Murali Karicheri, Sergei Shtylyov
In-Reply-To: <d6655497-5246-c24e-de35-fc6acdad0bf1@gmail.com>
> On Jun 20, 2019, at 3:47 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 6/19/19 10:40 AM, Vedang Patel wrote:
>> skb->tstamp is being used at multiple places. On the transmit side, it
>> is used to determine the launchtime of the packet. It is also used to
>> determine the software timestamp after the packet has been transmitted.
>>
>> So, clear out the tstamp value after it has been read so that we do not
>> report false software timestamp on the receive side.
>>
>> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
>> ---
>> drivers/net/ethernet/intel/igb/igb_main.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>> index fc925adbd9fa..f66dae72fe37 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -5688,6 +5688,7 @@ static void igb_tx_ctxtdesc(struct igb_ring *tx_ring,
>> */
>> if (tx_ring->launchtime_enable) {
>> ts = ns_to_timespec64(first->skb->tstamp);
>> + first->skb->tstamp = 0;
>
> Please provide more explanations.
>
> Why only this driver would need this ?
>
Currently, igb is the only driver which uses the skb->tstamp option on the transmit side (to set the hardware transmit timestamp). All the other drivers only use it on the receive side (to collect and send the hardware transmit timestamp to the userspace after packet has been sent).
So, any driver which supports the hardware txtime in the future will have to clear skb->tstamp to make sure that hardware tx transmit and tx timestamping can be done on the same packet.
Thanks,
Vedang
>
>> context_desc->seqnum_seed = cpu_to_le32(ts.tv_nsec / 32);
>> } else {
>> context_desc->seqnum_seed = 0;
>>
^ permalink raw reply
* [PATCH 00/11] XDP unaligned chunk placement support
From: Kevin Laatz @ 2019-06-20 8:39 UTC (permalink / raw)
To: netdev, ast, daniel, bjorn.topel, magnus.karlsson
Cc: bpf, intel-wired-lan, bruce.richardson, ciara.loftus, Kevin Laatz
This patchset adds the ability to use unaligned chunks in the XDP umem.
Currently, all chunk addresses passed to the umem are masked to be chunk
size aligned (default is 2k, max is PAGE_SIZE). This limits where we can
place chunks within the umem as well as limiting the packet sizes that are
supported.
The changes in this patchset removes these restrictions, allowing XDP to be
more flexible in where it can place a chunk within a umem. By relaxing where
the chunks can be placed, it allows us to use an arbitrary buffer size and
place that wherever we have a free address in the umem. These changes add the
ability to support jumboframes and make it easy to integrate with other
existing frameworks that have their own memory management systems, such as
DPDK.
Structure of the patchset:
Patch 1:
- Remove unnecessary masking and headroom addition during zero-copy Rx
buffer recycling in i40e. This change is required in order for the
buffer recycling to work in the unaligned chunk mode.
Patch 2:
- Remove unnecessary masking and headroom addition during
zero-copy Rx buffer recycling in ixgbe. This change is required in
order for the buffer recycling to work in the unaligned chunk mode.
Patch 3:
- Adds an offset parameter to zero_copy_allocator. This change will
enable us to calculate the original handle in zca_free. This will be
required for unaligned chunk mode since we can't easily mask back to
the original handle.
Patch 4:
- Adds the offset parameter to i40e_zca_free. This change is needed for
calculating the handle since we can't easily mask back to the original
handle like we can in the aligned case.
Patch 5:
- Adds the offset parameter to ixgbe_zca_free. This change is needed for
calculating the handle since we can't easily mask back to the original
handle like we can in the aligned case.
Patch 6:
- Add infrastructure for unaligned chunks. Since we are dealing
with unaligned chunks that could potentially cross a physical page
boundary, we add checks to keep track of that information. We can
later use this information to correctly handle buffers that are
placed at an address where they cross a page boundary.
Patch 7:
- Add flags for umem configuration to libbpf
Patch 8:
- Modify xdpsock application to add a command line option for
unaligned chunks
Patch 9:
- Addition of command line argument to pass in a desired buffer size
and buffer recycling for unaligned mode. Passing in a buffer size will
allow the application to use unaligned chunks with the unaligned chunk
mode. Since we are now using unaligned chunks, we need to recycle our
buffers in a slightly different way.
Patch 10:
- Adds hugepage support to the xdpsock application
Patch 11:
- Documentation update to include the unaligned chunk scenario. We need
to explicitly state that the incoming addresses are only masked in the
aligned chunk mode and not the unaligned chunk mode.
Kevin Laatz (11):
i40e: simplify Rx buffer recycle
ixgbe: simplify Rx buffer recycle
xdp: add offset param to zero_copy_allocator
i40e: add offset to zca_free
ixgbe: add offset to zca_free
xsk: add support to allow unaligned chunk placement
libbpf: add flags to umem config
samples/bpf: add unaligned chunks mode support to xdpsock
samples/bpf: add buffer recycling for unaligned chunks to xdpsock
samples/bpf: use hugepages in xdpsock app
doc/af_xdp: include unaligned chunk case
Documentation/networking/af_xdp.rst | 10 +-
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 21 ++--
drivers/net/ethernet/intel/i40e/i40e_xsk.h | 3 +-
.../ethernet/intel/ixgbe/ixgbe_txrx_common.h | 3 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 21 ++--
include/net/xdp.h | 3 +-
include/net/xdp_sock.h | 2 +
include/uapi/linux/if_xdp.h | 4 +
net/core/xdp.c | 11 ++-
net/xdp/xdp_umem.c | 17 ++--
net/xdp/xsk.c | 60 +++++++++--
net/xdp/xsk_queue.h | 60 +++++++++--
samples/bpf/xdpsock_user.c | 99 ++++++++++++++-----
tools/include/uapi/linux/if_xdp.h | 4 +
tools/lib/bpf/xsk.c | 7 ++
tools/lib/bpf/xsk.h | 2 +
16 files changed, 241 insertions(+), 86 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH 01/11] i40e: simplify Rx buffer recycle
From: Kevin Laatz @ 2019-06-20 8:39 UTC (permalink / raw)
To: netdev, ast, daniel, bjorn.topel, magnus.karlsson
Cc: bpf, intel-wired-lan, bruce.richardson, ciara.loftus, Kevin Laatz
In-Reply-To: <20190620083924.1996-1-kevin.laatz@intel.com>
Currently, the dma, addr and handle are modified when we reuse Rx buffers
in zero-copy mode. However, this is not required as the inputs to the
function are copies, not the original values themselves. As we use the
copies within the function, we can use the original 'old_bi' values
directly without having to mask and add the headroom.
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 1b17486543ac..c89e692e8663 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -419,8 +419,6 @@ static void i40e_reuse_rx_buffer_zc(struct i40e_ring *rx_ring,
struct i40e_rx_buffer *old_bi)
{
struct i40e_rx_buffer *new_bi = &rx_ring->rx_bi[rx_ring->next_to_alloc];
- unsigned long mask = (unsigned long)rx_ring->xsk_umem->chunk_mask;
- u64 hr = rx_ring->xsk_umem->headroom + XDP_PACKET_HEADROOM;
u16 nta = rx_ring->next_to_alloc;
/* update, and store next to alloc */
@@ -428,14 +426,9 @@ static void i40e_reuse_rx_buffer_zc(struct i40e_ring *rx_ring,
rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
/* transfer page from old buffer to new buffer */
- new_bi->dma = old_bi->dma & mask;
- new_bi->dma += hr;
-
- new_bi->addr = (void *)((unsigned long)old_bi->addr & mask);
- new_bi->addr += hr;
-
- new_bi->handle = old_bi->handle & mask;
- new_bi->handle += rx_ring->xsk_umem->headroom;
+ new_bi->dma = old_bi->dma;
+ new_bi->addr = old_bi->addr;
+ new_bi->handle = old_bi->handle;
old_bi->addr = NULL;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 02/11] ixgbe: simplify Rx buffer recycle
From: Kevin Laatz @ 2019-06-20 8:39 UTC (permalink / raw)
To: netdev, ast, daniel, bjorn.topel, magnus.karlsson
Cc: bpf, intel-wired-lan, bruce.richardson, ciara.loftus, Kevin Laatz
In-Reply-To: <20190620083924.1996-1-kevin.laatz@intel.com>
Currently, the dma, addr and handle are modified when we reuse Rx buffers
in zero-copy mode. However, this is not required as the inputs to the
function are copies, not the original values themselves. As we use the
copies within the function, we can use the original 'obi' values
directly without having to mask and add the headroom.
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index bfe95ce0bd7f..49536adafe8e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -251,8 +251,6 @@ ixgbe_rx_buffer *ixgbe_get_rx_buffer_zc(struct ixgbe_ring *rx_ring,
static void ixgbe_reuse_rx_buffer_zc(struct ixgbe_ring *rx_ring,
struct ixgbe_rx_buffer *obi)
{
- unsigned long mask = (unsigned long)rx_ring->xsk_umem->chunk_mask;
- u64 hr = rx_ring->xsk_umem->headroom + XDP_PACKET_HEADROOM;
u16 nta = rx_ring->next_to_alloc;
struct ixgbe_rx_buffer *nbi;
@@ -262,14 +260,9 @@ static void ixgbe_reuse_rx_buffer_zc(struct ixgbe_ring *rx_ring,
rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
/* transfer page from old buffer to new buffer */
- nbi->dma = obi->dma & mask;
- nbi->dma += hr;
-
- nbi->addr = (void *)((unsigned long)obi->addr & mask);
- nbi->addr += hr;
-
- nbi->handle = obi->handle & mask;
- nbi->handle += rx_ring->xsk_umem->headroom;
+ nbi->dma = obi->dma;
+ nbi->addr = obi->addr;
+ nbi->handle = obi->handle;
obi->addr = NULL;
obi->skb = NULL;
--
2.17.1
^ permalink raw reply related
* [PATCH 03/11] xdp: add offset param to zero_copy_allocator
From: Kevin Laatz @ 2019-06-20 8:39 UTC (permalink / raw)
To: netdev, ast, daniel, bjorn.topel, magnus.karlsson
Cc: bpf, intel-wired-lan, bruce.richardson, ciara.loftus, Kevin Laatz
In-Reply-To: <20190620083924.1996-1-kevin.laatz@intel.com>
This patch adds an offset parameter for zero_copy_allocator.
This change is required for the unaligned chunk mode which will come later
in this patch set. The offset parameter is required for calculating the
original handle in unaligned mode since we can't easily mask back to it
like in the aligned case.
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
include/net/xdp.h | 3 ++-
net/core/xdp.c | 11 ++++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 0f25b3675c5c..ea801fd2bf98 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -53,7 +53,8 @@ struct xdp_mem_info {
struct page_pool;
struct zero_copy_allocator {
- void (*free)(struct zero_copy_allocator *zca, unsigned long handle);
+ void (*free)(struct zero_copy_allocator *zca, unsigned long handle,
+ off_t off);
};
struct xdp_rxq_info {
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 4b2b194f4f1f..a77a7162d213 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -322,7 +322,7 @@ EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
* of xdp_frames/pages in those cases.
*/
static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
- unsigned long handle)
+ unsigned long handle, off_t off)
{
struct xdp_mem_allocator *xa;
struct page *page;
@@ -353,7 +353,7 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
rcu_read_lock();
/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
- xa->zc_alloc->free(xa->zc_alloc, handle);
+ xa->zc_alloc->free(xa->zc_alloc, handle, off);
rcu_read_unlock();
default:
/* Not possible, checked in xdp_rxq_info_reg_mem_model() */
@@ -363,19 +363,20 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
void xdp_return_frame(struct xdp_frame *xdpf)
{
- __xdp_return(xdpf->data, &xdpf->mem, false, 0);
+ __xdp_return(xdpf->data, &xdpf->mem, false, 0, 0);
}
EXPORT_SYMBOL_GPL(xdp_return_frame);
void xdp_return_frame_rx_napi(struct xdp_frame *xdpf)
{
- __xdp_return(xdpf->data, &xdpf->mem, true, 0);
+ __xdp_return(xdpf->data, &xdpf->mem, true, 0, 0);
}
EXPORT_SYMBOL_GPL(xdp_return_frame_rx_napi);
void xdp_return_buff(struct xdp_buff *xdp)
{
- __xdp_return(xdp->data, &xdp->rxq->mem, true, xdp->handle);
+ __xdp_return(xdp->data, &xdp->rxq->mem, true, xdp->handle,
+ xdp->data - xdp->data_hard_start);
}
EXPORT_SYMBOL_GPL(xdp_return_buff);
--
2.17.1
^ permalink raw reply related
* [PATCH 04/11] i40e: add offset to zca_free
From: Kevin Laatz @ 2019-06-20 8:39 UTC (permalink / raw)
To: netdev, ast, daniel, bjorn.topel, magnus.karlsson
Cc: bpf, intel-wired-lan, bruce.richardson, ciara.loftus, Kevin Laatz
In-Reply-To: <20190620083924.1996-1-kevin.laatz@intel.com>
This patch adds the offset param to for zero_copy_allocator to
i40e_zca_free. This change is required to calculate the handle, otherwise,
this function will not work in unaligned chunk mode since we can't easily mask
back to the original handle in unaligned chunk mode.
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 8 ++++----
drivers/net/ethernet/intel/i40e/i40e_xsk.h | 3 ++-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index c89e692e8663..8c281f356293 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -438,16 +438,16 @@ static void i40e_reuse_rx_buffer_zc(struct i40e_ring *rx_ring,
* @alloc: Zero-copy allocator
* @handle: Buffer handle
**/
-void i40e_zca_free(struct zero_copy_allocator *alloc, unsigned long handle)
+void i40e_zca_free(struct zero_copy_allocator *alloc, unsigned long handle,
+ off_t off)
{
struct i40e_rx_buffer *bi;
struct i40e_ring *rx_ring;
- u64 hr, mask;
+ u64 hr;
u16 nta;
rx_ring = container_of(alloc, struct i40e_ring, zca);
hr = rx_ring->xsk_umem->headroom + XDP_PACKET_HEADROOM;
- mask = rx_ring->xsk_umem->chunk_mask;
nta = rx_ring->next_to_alloc;
bi = &rx_ring->rx_bi[nta];
@@ -455,7 +455,7 @@ void i40e_zca_free(struct zero_copy_allocator *alloc, unsigned long handle)
nta++;
rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
- handle &= mask;
+ handle -= off;
bi->dma = xdp_umem_get_dma(rx_ring->xsk_umem, handle);
bi->dma += hr;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.h b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
index 8cc0a2e7d9a2..85691dc9ac42 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
@@ -12,7 +12,8 @@ int i40e_queue_pair_disable(struct i40e_vsi *vsi, int queue_pair);
int i40e_queue_pair_enable(struct i40e_vsi *vsi, int queue_pair);
int i40e_xsk_umem_setup(struct i40e_vsi *vsi, struct xdp_umem *umem,
u16 qid);
-void i40e_zca_free(struct zero_copy_allocator *alloc, unsigned long handle);
+void i40e_zca_free(struct zero_copy_allocator *alloc, unsigned long handle,
+ off_t off);
bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 cleaned_count);
int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget);
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox