Netdev List
 help / color / mirror / Atom feed
* Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
From: Willem de Bruijn @ 2018-07-06 22:16 UTC (permalink / raw)
  To: syzbot+7b9ed9872dab8c32305d
  Cc: David Miller, Alexey Kuznetsov, LKML, Network Development,
	syzkaller-bugs, Hideaki YOSHIFUJI
In-Reply-To: <00000000000033f8440570585207@google.com>

On Fri, Jul 6, 2018 at 1:55 PM syzbot
<syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com> wrote:
>
> syzbot has found a reproducer for the following crash on:
>
> HEAD commit:    70ba5b6db96f ipv4: Return EINVAL when ping_group_range sys..
> git tree:       net
> console output: https://syzkaller.appspot.com/x/log.txt?x=13cd2970400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
> dashboard link: https://syzkaller.appspot.com/bug?extid=7b9ed9872dab8c32305d
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15dfb748400000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12a1050c400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com
>
> IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
> 8021q: adding VLAN 0 to HW filter on device team0
> ==================================================================
> BUG: KASAN: use-after-free in ipv6_gso_pull_exthdrs+0x57a/0x5f0
> net/ipv6/ip6_offload.c:45
> Read of size 1 at addr ffff8801ce17f3a9 by task syz-executor655/4567

This is an 8 byte packet with a virtio_net_hdr describing it as
VIRTIO_NET_HDR_GSO_TCPV4, but with protocol ETH_P_NSH. That
matches the occurrence of nsh_gso_segment in the stack trace.

This pulls the struct nshhdr of 8B, passing a packet with skb->len 0
to skb_mac_gso_segment. That is the only GRO function that
unconditionally calls _skb_pull without first checking pskb_may_pull.
Adding that check does catch this:

+       if (unlikely(!pskb_may_pull(skb, vlan_depth)))
+               return ERR_PTR(-EINVAL);

^ permalink raw reply

* Re: [PATCH v12 01/10] dt-bindings: Add Cavium Octeon Common Ethernet Interface.
From: Andrew Lunn @ 2018-07-06 22:41 UTC (permalink / raw)
  To: Steven J. Hill; +Cc: netdev, Chandrakala Chavva
In-Reply-To: <d34bed40-5b2a-8a89-f86d-6f54ae3ecdc3@caviumnetworks.com>

On Fri, Jul 06, 2018 at 05:10:39PM -0500, Steven J. Hill wrote:
> On 06/28/2018 03:35 AM, Andrew Lunn wrote:
> > 
> >> +- cavium,rx-clk-delay-bypass: Set to <1> to bypass the rx clock delay setting.
> >> +  Needed by the Micrel PHY.
> > 
> > Could you explain this some more. Is it anything to do with RGMII delays?
> > 
> Andrew,
> 
> One of my colleagues tracked this down for me. This device tree option is in place
> because there are several different ways to do the clock and data with respect to
> RGMII. This controls the delay introduced for the RX clock with respect to the data.
> Without this, RX will not work with Micrel PHYs. Thanks.

Hi Steven

This is his RGMII delays, as i guess.

Don't add this property, do it the Linux way. Look at phy-mode values

phy.h:	  PHY_INTERFACE_MODE_RGMII_ID,
phy.h:	  PHY_INTERFACE_MODE_RGMII_RXID,
phy.h:	  PHY_INTERFACE_MODE_RGMII_TXID,

There are plenty of examples in drivers/net/ethernet

      Andrew

^ permalink raw reply

* Re: Crash due to destroying TCP request sockets using SOCK_DESTROY
From: Subash Abhinov Kasiviswanathan @ 2018-07-06 23:24 UTC (permalink / raw)
  To: Eric Dumazet, Lorenzo Colitti; +Cc: netdev, Alistair Strachan, David Ahern
In-Reply-To: <af52b285-e97b-f610-acc6-cb1361609034@gmail.com>

>> Looks like for a TCP_NEW_SYN_RECV socket, sock_diag_destroy
>> essentially ends up doing:
>> 
>>                         struct request_sock *req = inet_reqsk(sk);
>> 
>>                         local_bh_disable();
>>                         
>> inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
>>                                                           req);
>>                         local_bh_enable();
>> ...
>> 
>>         sock_gen_put(sk);
>> 
>> It looks like inet_csk_reqsk_queue_drop_and_put calls reqsk_put(req),
>> which frees the socket, and at that point sock_gen_put is a UAF. Do we
>> just need:
>> 
>> -                        
>> inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
>> -                                                           req);
>> +                        inet_csk_reqsk_queue_drop(req->rsk_listener, 
>> req);
>> 
>> since sock_gen_put will also end up calling reqsk_put() for a
>> TCP_SYN_RECV socket?
>> 
>> Alastair - you're able to reproduce this UAF using net_test on qemu,
>> right? If so, could you try that two-line patch above?
>> 
> 
> Hi Lorenzo
> 
> Your patch makes sense to me, please submit it formally with :
> 
> Fixes: d7226c7a4dd1 ("net: diag: Fix refcnt leak in error path
> destroying socket")
> Cc: David Ahern <dsa@cumulusnetworks.com>
> 
> Thanks !

Thanks Lorenzo and Eric. I will try it out locally.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH net 1/3] net/ipv6: fix addrconf_sysctl_addr_gen_mode
From: David Ahern @ 2018-07-06 23:28 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: netdev, Jiri Pirko, Felix Jia
In-Reply-To: <20180706155857.GA8053@bistromath.localdomain>

On 7/6/18 9:58 AM, Sabrina Dubroca wrote:
> 
> Right. I'll add that as a separate patch in this series, unless you
> really prefer the change squashed into this patch.

no preference.

> 
> 
>> Looking at other addr_gen_mode sites, addrconf_sysctl_stable_secret is
>> messed up as well. It propagates a change to 'default' to all existing
>> devices.
> 
> I guess it was intentional, given:
> 
>         if (&net->ipv6.devconf_all->stable_secret == ctl->data)
>                 return -EIO;
> 
> It only propagates the mode, and not the secret itself, to all
> devices. After thinking about it for a while, I guess it considers the
> new default not only as default for newly created devices, but also
> for newly added addresses/prefixes.
> Or am I making stuff up?
> 

Maybe Hannes can explain (622c81d57b392). It should have been all
instead of default. As I understand it default is what devices start
with on create and individual devices can be changed. 'All' overrides
device-specific settings. So the above is inconsistent. One of many with
sysctl that makes it frustrating for users.

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Alexei Starovoitov @ 2018-07-06 23:38 UTC (permalink / raw)
  To: Waskiewicz Jr, Peter
  Cc: jakub.kicinski@netronome.com, Duyck, Alexander H,
	daniel@iogearbox.net, saeedm@mellanox.com, brouer@redhat.com,
	borkmann@iogearbox.net, tariqt@mellanox.com,
	john.fastabend@gmail.com, netdev@vger.kernel.org,
	saeedm@dev.mellanox.co.il
In-Reply-To: <1530909863.4291.12.camel@intel.com>

On Fri, Jul 06, 2018 at 08:44:24PM +0000, Waskiewicz Jr, Peter wrote:
> On Fri, 2018-07-06 at 09:30 -0700, Alexei Starovoitov wrote:
> > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:
> > > 
> > > I'm also not 100% on board with the argument that "future" FW can
> > > reshuffle things whatever way it wants to.  Is the assumption that
> > > future ASICs/FW will be designed to always use the "blessed" BTF
> > > format?  Or will it be reconfigurable at runtime?
> > 
> > let's table configuration of metadata aside for a second.
> 
> I agree that this should/could be NIC-specific and shouldn't weigh on
> the metadata interface between the drivers and XDP layer.
> 
> > Describing metedata layout in BTF allows NICs to disclose everything
> > NIC has to users in a standard and generic way.
> > Whether firmware is reconfigurable on the fly or has to reflashed
> > and hw powercycled to have new md layout (and corresponding BTF
> > description)
> > is a separate discussion.
> > Saeed's proposal introduces the concept of 'offset' inside 'struct
> > xdp_md_info'
> > to reach 'hash' value in metadata.
> > Essentially it's a run-time way to access 'hash' instead of build-
> > time.
> > So bpf program would need two loads to read csum or hash field
> > instead of one.
> > With BTF the layout of metadata is known to the program at build-
> > time.
> > 
> > To reiterate the proposal:
> > - driver+firmware keep layout of the metadata in BTF format (either
> > in the driver
> >   or driver can read it from firmware)
> > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will query
> > the driver and
> >   generate normal C header file based on BTF in the given NIC
> > - user does #include "md_desc.h" and bpf program can access md->csum
> > or md->hash
> >   with direct single load out of metadata area in front of the packet
> 
> This piece is where I'd like to discuss more.  When we discussed this
> in Seoul, the initial proposal was a static struct that we'd try to
> hammer out a common layout between the interested parties.  That
> obviously wasn't going to scale, and we wanted to pursue something more
> dynamic.  But I thought the goal was the XDP/eBPF program wouldn't want
> to care what the underlying device is, and could just ask for metadata
> that it's interested in.  With this approach, your eBPF program is now
> bound/tied to the NIC/driver, and if you switched to a differen
> NIC/driver combo, then you'd have to rewrite part of your eBPF program
> to comprehend that.  I thought we were trying to avoid that.

It looks to me that NICs have a lot more differences instead of common pieces.
If we focus the architecture on making common things as generic
as possible we miss the bigger picture of covering distinct
and unique features that hw provides.

In the last email when I said "would be good to standardize at least a few common fields"
I meant that it make sense only at the later phases.
I don't think we should put things into uapi upfront.
Otherwise we will end up with likely useless fields.
Like vlan field. Surely a lot of nics can store it into metadata, but why?
bpf program can easily read it from the packet.
What's the benefit of adding it to md?
This metadata concept we're discussing in the context of program acceleration.
If metadata doesn't give perf benefits it should not be done by
firmware, it should not be supported by the driver and certainly
should not be part of uapi.
Hence I'd like to see pure BTF description without any common/standard
fields across the drivers and figure out what (if anything) is useful
to accelerate xdp programs (and af_xdp in the future).

> Our proposed approach (still working on something ready to RFC) is to
> provide a method for the eBPF program to send a struct of requested
> hints down to the driver on load.  If the driver can provide the hints,
> then that'd be how they'd be laid out in the metadata.  If it can't
> provide them, we'd probably reject the program loading, or discuss
> providing a software fallback (I know this is an area of contention).

I don't think that approach can work.
My .02 is the only useful acceleration feature out if intel nics is
an ability to parse the packet and report it to the xdp program
as a node id in the parse graph. As far as I know that is pretty unique
to intel. Configuration of that is a different matter.
Other things like hash are interesting, but we will quickly get
into rss spec definition if we go standardization route now instead
of later phases.
Therefore I'd rather let firmware+driver define its own BTF description
that says here is 'hash' of the packet hw can provide and it's fine
that this hash may be over different tuples depending on the nic
and even version of the firmware in that nic.
We need to see performance numbers and benefits for real world
xdp programs before drilling into specific semantics of the hash.

> I suppose we could get there with the rewriting mechanism described
> below, but that'd be a tough sell to set a bit of ABI for metadata,
> then change it to be potentially dynamic at runtime in the future.

Hmm. I don't see how future offset rewriting will break abi.
It looks to me as natural extension that wouldn't break any apps
that would be written for specific nic with given BTF description.
With rewritting in place some progs would become portable. That's all.

^ permalink raw reply

* Re: [PATCH v3 bpf-next 1/3] bpftool: introduce cgroup tree command
From: Daniel Borkmann @ 2018-07-06 23:40 UTC (permalink / raw)
  To: Roman Gushchin, netdev
  Cc: linux-kernel, kernel-team, Jakub Kicinski, Quentin Monnet,
	Alexei Starovoitov
In-Reply-To: <20180706212816.3760-1-guro@fb.com>

On 07/06/2018 11:28 PM, Roman Gushchin wrote:
> This commit introduces a new bpftool command: cgroup tree.
> The idea is to iterate over the whole cgroup tree and print
> all attached programs.
> 
> I was debugging a bpf/systemd issue, and found, that there is
> no simple way to listen all bpf programs attached to cgroups.
> I did master something in bash, but after some time got tired of it,
> and decided, that adding a dedicated bpftool command could be
> a better idea.
> 
> So, here it is:
>   $ sudo ./bpftool cgroup tree
>   CgroupPath
>   ID       AttachType      AttachFlags     Name
>   /sys/fs/cgroup/system.slice/systemd-machined.service
>       18       ingress
>       17       egress
>   /sys/fs/cgroup/system.slice/systemd-logind.service
>       20       ingress
>       19       egress
>   /sys/fs/cgroup/system.slice/systemd-udevd.service
>       16       ingress
>       15       egress
>   /sys/fs/cgroup/system.slice/systemd-journald.service
>       14       ingress
>       13       egress
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>

Very useful! Applied to bpf-next, thanks everyone!

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Alexei Starovoitov @ 2018-07-06 23:42 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Daniel Borkmann, Saeed Mahameed, saeedm@dev.mellanox.co.il,
	alexander.h.duyck@intel.com, netdev@vger.kernel.org, Tariq Toukan,
	john.fastabend@gmail.com, brouer@redhat.com,
	borkmann@iogearbox.net, peter.waskiewicz.jr@intel.com
In-Reply-To: <20180706143358.0240ff66@cakuba.netronome.com>

On Fri, Jul 06, 2018 at 02:33:58PM -0700, Jakub Kicinski wrote:
> On Fri, 6 Jul 2018 09:30:42 -0700, Alexei Starovoitov wrote:
> > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:
> > > 
> > > I'm also not 100% on board with the argument that "future" FW can
> > > reshuffle things whatever way it wants to.  Is the assumption that
> > > future ASICs/FW will be designed to always use the "blessed" BTF
> > > format?  Or will it be reconfigurable at runtime?  
> > 
> > let's table configuration of metadata aside for a second.
> > 
> > Describing metedata layout in BTF allows NICs to disclose everything
> > NIC has to users in a standard and generic way.
> > Whether firmware is reconfigurable on the fly or has to reflashed
> > and hw powercycled to have new md layout (and corresponding BTF description)
> > is a separate discussion.
> > Saeed's proposal introduces the concept of 'offset' inside 'struct xdp_md_info'
> > to reach 'hash' value in metadata.
> > Essentially it's a run-time way to access 'hash' instead of build-time.
> > So bpf program would need two loads to read csum or hash field instead of one.
> > With BTF the layout of metadata is known to the program at build-time.
> > 
> > To reiterate the proposal:
> > - driver+firmware keep layout of the metadata in BTF format (either in the driver
> >   or driver can read it from firmware)
> > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will query the driver and
> >   generate normal C header file based on BTF in the given NIC
> > - user does #include "md_desc.h" and bpf program can access md->csum or md->hash
> >   with direct single load out of metadata area in front of the packet
> > - llvm compiles bpf program and records how program is doing this md->csum accesses
> >   in BTF format as well (the compiler will be keeping such records
> >   for __sk_buff and all other structs too, but that's separate discussion)
> > - during sys_bpf(prog_load) the kernel checks (via supplied BTF) that the way the program
> >   accesses metadata (and other structs) matches BTF from the driver,
> >   so no surprises if driver+firmware got updated, but program is not recompiled
> > - every NIC can have their own layout of metadata and its own meaning of the fields,
> >   but would be good to standardize at least a few common fields like hash
> 
> Can I expose HW descriptors this way, though, or is the proposal to
> copy this data into the packet buffer?

That crossed my mind too. We can use BTF to describe HW descriptors too,
but I don't think it would buy us anything. AF_XDP approach is better.

> > Once this is working we can do more cool things with BTF.
> > Like doing offset rewriting at program load time similar to what we plan
> > to do for tracing. Tracing programs will be doing 'task->pid' access
> > and the kernel will adjust offsetof(struct task_struct, pid) during program load
> > depending on BTF for the kernel.
> > The same trick we can do for networking metadata.
> > The program will contain load instruction md->hash that will get automatically
> > adjusted to proper offset depending on BTF of 'hash' field in the NIC.
> > For now I'm proposing _not_ to go that far with offset rewriting and start
> > with simple steps described above.
> 
> Why? :(  Could we please go with the rewrite/driver helpers instead of
> impacting fast paths of the drivers yet again?  This rewrite should be
> easier than task->pid, because we have the synthetic user space struct
> xdp_md definition.

I don't understand 'impact fast path yet again' concern.
If NIC has certain metadata today, just derscribe what it has in BTF
and supply the actual per-packet md to xdp program as-is.
No changes for fast path at all.
Future rewritting will be done by the bpf/xdp core with zero
changes to the driver. All driver has to do is to provide BTF.

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/6] nfp: bpf: add multiplication and divide support on NFP JIT
From: Daniel Borkmann @ 2018-07-06 23:49 UTC (permalink / raw)
  To: Jakub Kicinski, alexei.starovoitov; +Cc: Song Liu, oss-drivers, netdev
In-Reply-To: <20180706221323.31688-1-jakub.kicinski@netronome.com>

On 07/07/2018 12:13 AM, Jakub Kicinski wrote:
> Jiong says:
> 
> NFP supports u16 and u32 multiplication. Multiplication is done 8-bits per
> step, therefore we need 2 steps for u16 and 4 steps for u32.
> 
> We also need one start instruction to initialize the sequence and one or
> two instructions to fetch the result depending on either you need the high
> halve of u32 multiplication.
> 
> For ALU64, if either operand is beyond u32's value range, we reject it. One
> thing to note, if the source operand is BPF_K, then we need to check "imm"
> field directly, and we'd reject it if it is negative.  Because for ALU64,
> "imm" (with s32 type) is expected to be sign extended to s64 which NFP mul
> doesn't support. For ALU32, it is fine for "imm" be negative though,
> because the result is 32-bits and here is no difference on the low halve
> of result for signed/unsigned mul, so we will get correct result.
> 
> NFP doesn't have integer divide instruction, this patch set uses reciprocal
> algorithm (the basic one, reciprocal_div) to emulate it.
> 
> For each u32 divide, we would need 11 instructions to finish the operation.
> 
>    7 (for multiplication) + 4 (various ALUs) = 11
> 
> Given NFP only supports multiplication no bigger than u32, we'd require
> divisor and dividend no bigger than that as well.
> 
> Also eBPF doesn't support signed divide and has enforced this on C language
> level by failing compilation. However LLVM assembler hasn't enforced this,
> so it is possible for negative constant to leak in as a BPF_K operand
> through assembly code, we reject such cases as well.
> 
> Meanwhile reciprocal_div.h only implemented the basic version of:
> 
>    "Division by Invariant Integers Using Multiplication"
>                           - Torbjörn Granlund and Peter L. Montgomery
> 
> This patch set further implements the optimized version (Figure 4.2 in the
> paper) inside existing reciprocal_div.h. When the divider is even and the
> calculated reciprocal magic number doesn't fit u32, we could reduce the
> required ALU instructions from 4 to 2 or 1 for some cases.
> 
> The advanced version requires more complex calculation to get the
> reciprocal multiplier and other control variables, but then could reduce
> the required emulation operations. It makes sense to use it for JIT divide
> code generation (for example eBPF JIT backends) for which we are willing to
> trade performance of JITed code with that of host.
> 
> v2:
>   - add warning in l == 32 code path. (Song Liu/Jakub)
>   - jit separate insn sequence for l == 32. (Jakub/Edwin)
>   - should use unrestricted operand for mul.
>   - once place should use "1U << exp" instead of "1 << exp".
> 	    
> Jiong Wang (6):
>   lib: reciprocal_div: implement the improved algorithm on the paper
>     mentioned
>   nfp: bpf: rename umin/umax to umin_src/umax_src
>   nfp: bpf: copy range info for all operands of all ALU operations
>   nfp: bpf: support u16 and u32 multiplications
>   nfp: bpf: support u32 divide using reciprocal_div.h
>   nfp: bpf: migrate to advanced reciprocal divide in reciprocal_div.h

Applied to bpf-next, thanks everyone!

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Waskiewicz Jr, Peter @ 2018-07-06 23:49 UTC (permalink / raw)
  To: alexei.starovoitov@gmail.com
  Cc: Duyck, Alexander H, daniel@iogearbox.net, saeedm@mellanox.com,
	brouer@redhat.com, borkmann@iogearbox.net, tariqt@mellanox.com,
	john.fastabend@gmail.com, jakub.kicinski@netronome.com,
	netdev@vger.kernel.org, saeedm@dev.mellanox.co.il
In-Reply-To: <20180706233819.hqpxvoht7ijb5zqe@ast-mbp.dhcp.thefacebook.com>

On Fri, 2018-07-06 at 16:38 -0700, Alexei Starovoitov wrote:
> On Fri, Jul 06, 2018 at 08:44:24PM +0000, Waskiewicz Jr, Peter wrote:
> > On Fri, 2018-07-06 at 09:30 -0700, Alexei Starovoitov wrote:
> > > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:
> > > > 
> > > > I'm also not 100% on board with the argument that "future" FW
> > > > can
> > > > reshuffle things whatever way it wants to.  Is the assumption
> > > > that
> > > > future ASICs/FW will be designed to always use the "blessed"
> > > > BTF
> > > > format?  Or will it be reconfigurable at runtime?
> > > 
> > > let's table configuration of metadata aside for a second.
> > 
> > I agree that this should/could be NIC-specific and shouldn't weigh
> > on
> > the metadata interface between the drivers and XDP layer.
> > 
> > > Describing metedata layout in BTF allows NICs to disclose
> > > everything
> > > NIC has to users in a standard and generic way.
> > > Whether firmware is reconfigurable on the fly or has to reflashed
> > > and hw powercycled to have new md layout (and corresponding BTF
> > > description)
> > > is a separate discussion.
> > > Saeed's proposal introduces the concept of 'offset' inside
> > > 'struct
> > > xdp_md_info'
> > > to reach 'hash' value in metadata.
> > > Essentially it's a run-time way to access 'hash' instead of
> > > build-
> > > time.
> > > So bpf program would need two loads to read csum or hash field
> > > instead of one.
> > > With BTF the layout of metadata is known to the program at build-
> > > time.
> > > 
> > > To reiterate the proposal:
> > > - driver+firmware keep layout of the metadata in BTF format
> > > (either
> > > in the driver
> > >   or driver can read it from firmware)
> > > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will
> > > query
> > > the driver and
> > >   generate normal C header file based on BTF in the given NIC
> > > - user does #include "md_desc.h" and bpf program can access md-
> > > >csum
> > > or md->hash
> > >   with direct single load out of metadata area in front of the
> > > packet
> > 
> > This piece is where I'd like to discuss more.  When we discussed
> > this
> > in Seoul, the initial proposal was a static struct that we'd try to
> > hammer out a common layout between the interested parties.  That
> > obviously wasn't going to scale, and we wanted to pursue something
> > more
> > dynamic.  But I thought the goal was the XDP/eBPF program wouldn't
> > want
> > to care what the underlying device is, and could just ask for
> > metadata
> > that it's interested in.  With this approach, your eBPF program is
> > now
> > bound/tied to the NIC/driver, and if you switched to a differen
> > NIC/driver combo, then you'd have to rewrite part of your eBPF
> > program
> > to comprehend that.  I thought we were trying to avoid that.
> 
> It looks to me that NICs have a lot more differences instead of
> common pieces.
> If we focus the architecture on making common things as generic
> as possible we miss the bigger picture of covering distinct
> and unique features that hw provides.
> 
> In the last email when I said "would be good to standardize at least
> a few common fields"
> I meant that it make sense only at the later phases.
> I don't think we should put things into uapi upfront.
> Otherwise we will end up with likely useless fields.
> Like vlan field. Surely a lot of nics can store it into metadata, but
> why?
> bpf program can easily read it from the packet.
> What's the benefit of adding it to md?
> This metadata concept we're discussing in the context of program
> acceleration.
> If metadata doesn't give perf benefits it should not be done by
> firmware, it should not be supported by the driver and certainly
> should not be part of uapi.
> Hence I'd like to see pure BTF description without any
> common/standard
> fields across the drivers and figure out what (if anything) is useful
> to accelerate xdp programs (and af_xdp in the future).

I guess I didn't write my response very well before, apologies.  I
actually really like the BTF description and the dynamic-ness of it. 
It solves lots of problems and keeps things out of the UAPI.

What I was getting at was using the BTF description and dumping a
header file to be used in the program to be loaded.  If we have an
application developer include that header, it will be the Intel BTF
description, or Mellanox BTF description, etc.  I'm fine with that, but
it makes your program not as portable, since you'd need to get a
different header and recompile it if you change the NIC.  Unless I'm
missing something.

> 
> > Our proposed approach (still working on something ready to RFC) is
> > to
> > provide a method for the eBPF program to send a struct of requested
> > hints down to the driver on load.  If the driver can provide the
> > hints,
> > then that'd be how they'd be laid out in the metadata.  If it can't
> > provide them, we'd probably reject the program loading, or discuss
> > providing a software fallback (I know this is an area of
> > contention).
> 
> I don't think that approach can work.
> My .02 is the only useful acceleration feature out if intel nics is
> an ability to parse the packet and report it to the xdp program
> as a node id in the parse graph. As far as I know that is pretty
> unique
> to intel. Configuration of that is a different matter.
> Other things like hash are interesting, but we will quickly get
> into rss spec definition if we go standardization route now instead
> of later phases.
> Therefore I'd rather let firmware+driver define its own BTF
> description
> that says here is 'hash' of the packet hw can provide and it's fine
> that this hash may be over different tuples depending on the nic
> and even version of the firmware in that nic.
> We need to see performance numbers and benefits for real world
> xdp programs before drilling into specific semantics of the hash.

I agree that "useful" fields should be targeted.  I can see hash
getting more interesting though if one can specify what parts of the n-
tuples get included in the hash, or if we want hashes for inner packets
or outer headers if things are tunneled.  But that can be revisited
down the road.

I also agree that changing the configuration of the underlying parser
or pipeline to generate the metadata is a completely separate
discussion.

> 
> > I suppose we could get there with the rewriting mechanism described
> > below, but that'd be a tough sell to set a bit of ABI for metadata,
> > then change it to be potentially dynamic at runtime in the future.
> 
> Hmm. I don't see how future offset rewriting will break abi.
> It looks to me as natural extension that wouldn't break any apps
> that would be written for specific nic with given BTF description.
> With rewritting in place some progs would become portable. That's
> all.

What I meant here was if anything was put into the UAPI, then changing
to something more dynamic might make transitioning more challenging. 
And I can see now what the rewriting can do to help my previous
thoughts on the BTF description locking a program to a specific NIC. 
But I'd need to play with it to be convinced about the portability.

-PJ

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Jakub Kicinski @ 2018-07-07  0:08 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Saeed Mahameed, saeedm@dev.mellanox.co.il,
	alexander.h.duyck@intel.com, netdev@vger.kernel.org, Tariq Toukan,
	john.fastabend@gmail.com, brouer@redhat.com,
	borkmann@iogearbox.net, peter.waskiewicz.jr@intel.com
In-Reply-To: <20180706234249.dchucouomzwilytx@ast-mbp.dhcp.thefacebook.com>

On Fri, 6 Jul 2018 16:42:51 -0700, Alexei Starovoitov wrote:
> On Fri, Jul 06, 2018 at 02:33:58PM -0700, Jakub Kicinski wrote:
> > On Fri, 6 Jul 2018 09:30:42 -0700, Alexei Starovoitov wrote:  
> > > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:  
> > > > 
> > > > I'm also not 100% on board with the argument that "future" FW can
> > > > reshuffle things whatever way it wants to.  Is the assumption that
> > > > future ASICs/FW will be designed to always use the "blessed" BTF
> > > > format?  Or will it be reconfigurable at runtime?    
> > > 
> > > let's table configuration of metadata aside for a second.
> > > 
> > > Describing metedata layout in BTF allows NICs to disclose everything
> > > NIC has to users in a standard and generic way.
> > > Whether firmware is reconfigurable on the fly or has to reflashed
> > > and hw powercycled to have new md layout (and corresponding BTF description)
> > > is a separate discussion.
> > > Saeed's proposal introduces the concept of 'offset' inside 'struct xdp_md_info'
> > > to reach 'hash' value in metadata.
> > > Essentially it's a run-time way to access 'hash' instead of build-time.
> > > So bpf program would need two loads to read csum or hash field instead of one.
> > > With BTF the layout of metadata is known to the program at build-time.
> > > 
> > > To reiterate the proposal:
> > > - driver+firmware keep layout of the metadata in BTF format (either in the driver
> > >   or driver can read it from firmware)
> > > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will query the driver and
> > >   generate normal C header file based on BTF in the given NIC
> > > - user does #include "md_desc.h" and bpf program can access md->csum or md->hash
> > >   with direct single load out of metadata area in front of the packet
> > > - llvm compiles bpf program and records how program is doing this md->csum accesses
> > >   in BTF format as well (the compiler will be keeping such records
> > >   for __sk_buff and all other structs too, but that's separate discussion)
> > > - during sys_bpf(prog_load) the kernel checks (via supplied BTF) that the way the program
> > >   accesses metadata (and other structs) matches BTF from the driver,
> > >   so no surprises if driver+firmware got updated, but program is not recompiled
> > > - every NIC can have their own layout of metadata and its own meaning of the fields,
> > >   but would be good to standardize at least a few common fields like hash  
> > 
> > Can I expose HW descriptors this way, though, or is the proposal to
> > copy this data into the packet buffer?  
> 
> That crossed my mind too. We can use BTF to describe HW descriptors too,
> but I don't think it would buy us anything. AF_XDP approach is better.
>
> > > Once this is working we can do more cool things with BTF.
> > > Like doing offset rewriting at program load time similar to what we plan
> > > to do for tracing. Tracing programs will be doing 'task->pid' access
> > > and the kernel will adjust offsetof(struct task_struct, pid) during program load
> > > depending on BTF for the kernel.
> > > The same trick we can do for networking metadata.
> > > The program will contain load instruction md->hash that will get automatically
> > > adjusted to proper offset depending on BTF of 'hash' field in the NIC.
> > > For now I'm proposing _not_ to go that far with offset rewriting and start
> > > with simple steps described above.  
> > 
> > Why? :(  Could we please go with the rewrite/driver helpers instead of
> > impacting fast paths of the drivers yet again?  This rewrite should be
> > easier than task->pid, because we have the synthetic user space struct
> > xdp_md definition.  
> 
> I don't understand 'impact fast path yet again' concern.
> If NIC has certain metadata today, just derscribe what it has in BTF
> and supply the actual per-packet md to xdp program as-is.
> No changes for fast path at all.
> Future rewritting will be done by the bpf/xdp core with zero
> changes to the driver. All driver has to do is to provide BTF.

I'm confused.  AFAIK most *existing* NICs have the metadata in the
"descriptor", i.e. not in the packet buffer.  So if the NIC just
describes what it has, and there is no data shuffling/copying
(performance) then we have to expose the descriptor, no?

In case of NFP half of the metadata is in the packet buffer, half in
the descriptor.

Maybe I'm conflating your proposal with Saeed's.  In your proposal
would we still use the overload of metadata prepend (as in struct
xdp_md::data_meta, bpf_xdp_adjust_meta() etc.) or are we adding a
pointer to a new driver-defined struct inside struct xdp_md?

Thanks for bearing with me!

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Jakub Kicinski @ 2018-07-07  0:40 UTC (permalink / raw)
  To: Waskiewicz Jr, Peter
  Cc: alexei.starovoitov@gmail.com, Duyck, Alexander H,
	daniel@iogearbox.net, saeedm@mellanox.com, brouer@redhat.com,
	borkmann@iogearbox.net, tariqt@mellanox.com,
	john.fastabend@gmail.com, netdev@vger.kernel.org,
	saeedm@dev.mellanox.co.il
In-Reply-To: <1530920986.7664.5.camel@intel.com>

On Fri, 6 Jul 2018 23:49:55 +0000, Waskiewicz Jr, Peter wrote:
> On Fri, 2018-07-06 at 16:38 -0700, Alexei Starovoitov wrote:
> > On Fri, Jul 06, 2018 at 08:44:24PM +0000, Waskiewicz Jr, Peter wrote:  
> > > On Fri, 2018-07-06 at 09:30 -0700, Alexei Starovoitov wrote:  
> > > > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:  
> > > > > 
> > > > > I'm also not 100% on board with the argument that "future" FW
> > > > > can
> > > > > reshuffle things whatever way it wants to.  Is the assumption
> > > > > that
> > > > > future ASICs/FW will be designed to always use the "blessed"
> > > > > BTF
> > > > > format?  Or will it be reconfigurable at runtime?  
> > > > 
> > > > let's table configuration of metadata aside for a second.  
> > > 
> > > I agree that this should/could be NIC-specific and shouldn't weigh
> > > on
> > > the metadata interface between the drivers and XDP layer.
> > >   
> > > > Describing metedata layout in BTF allows NICs to disclose
> > > > everything
> > > > NIC has to users in a standard and generic way.
> > > > Whether firmware is reconfigurable on the fly or has to reflashed
> > > > and hw powercycled to have new md layout (and corresponding BTF
> > > > description)
> > > > is a separate discussion.
> > > > Saeed's proposal introduces the concept of 'offset' inside
> > > > 'struct
> > > > xdp_md_info'
> > > > to reach 'hash' value in metadata.
> > > > Essentially it's a run-time way to access 'hash' instead of
> > > > build-
> > > > time.
> > > > So bpf program would need two loads to read csum or hash field
> > > > instead of one.
> > > > With BTF the layout of metadata is known to the program at build-
> > > > time.
> > > > 
> > > > To reiterate the proposal:
> > > > - driver+firmware keep layout of the metadata in BTF format
> > > > (either
> > > > in the driver
> > > >   or driver can read it from firmware)
> > > > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will
> > > > query
> > > > the driver and
> > > >   generate normal C header file based on BTF in the given NIC
> > > > - user does #include "md_desc.h" and bpf program can access md-  
> > > > >csum  
> > > > or md->hash
> > > >   with direct single load out of metadata area in front of the
> > > > packet  
> > > 
> > > This piece is where I'd like to discuss more.  When we discussed
> > > this
> > > in Seoul, the initial proposal was a static struct that we'd try to
> > > hammer out a common layout between the interested parties.  That
> > > obviously wasn't going to scale, and we wanted to pursue something
> > > more
> > > dynamic.  But I thought the goal was the XDP/eBPF program wouldn't
> > > want
> > > to care what the underlying device is, and could just ask for
> > > metadata
> > > that it's interested in.  With this approach, your eBPF program is
> > > now
> > > bound/tied to the NIC/driver, and if you switched to a differen
> > > NIC/driver combo, then you'd have to rewrite part of your eBPF
> > > program
> > > to comprehend that.  I thought we were trying to avoid that.  
> > 
> > It looks to me that NICs have a lot more differences instead of
> > common pieces.
> > If we focus the architecture on making common things as generic
> > as possible we miss the bigger picture of covering distinct
> > and unique features that hw provides.
> > 
> > In the last email when I said "would be good to standardize at least
> > a few common fields"
> > I meant that it make sense only at the later phases.
> > I don't think we should put things into uapi upfront.
> > Otherwise we will end up with likely useless fields.
> > Like vlan field. Surely a lot of nics can store it into metadata, but
> > why?

FWIW vlan is extracted from the packet because of cBPF uABI.  tcpdump
breaks badly if VLAN header is in the packet buffer.  It may not be
entirely without merit to expose the field for writing, perhaps that can
simplify some EVPN use cases?  But that's most likely off-topic.

> > bpf program can easily read it from the packet.
> > What's the benefit of adding it to md?
> > This metadata concept we're discussing in the context of program
> > acceleration.
> > If metadata doesn't give perf benefits it should not be done by
> > firmware, it should not be supported by the driver and certainly
> > should not be part of uapi.
> > Hence I'd like to see pure BTF description without any
> > common/standard
> > fields across the drivers and figure out what (if anything) is useful
> > to accelerate xdp programs (and af_xdp in the future).  
> 
> I guess I didn't write my response very well before, apologies.  I
> actually really like the BTF description and the dynamic-ness of it. 
> It solves lots of problems and keeps things out of the UAPI.
> 
> What I was getting at was using the BTF description and dumping a
> header file to be used in the program to be loaded.  If we have an
> application developer include that header, it will be the Intel BTF
> description, or Mellanox BTF description, etc.  I'm fine with that, but
> it makes your program not as portable, since you'd need to get a
> different header and recompile it if you change the NIC.  Unless I'm
> missing something.

Could we just say that at the start we expose only existing SKB fields
(csum, hash, mark) and the meaning of the is the same as in the SKB?
That covers a lot of use cases and provides clear semantics (at least
to the driver developer) while naturally allowing to communicate the
fields from XDP to the stack with XDP_PASS?  It covers all Saeed did 
in his RFC, for example.

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Alexei Starovoitov @ 2018-07-07  0:45 UTC (permalink / raw)
  To: Waskiewicz Jr, Peter
  Cc: Duyck, Alexander H, daniel@iogearbox.net, saeedm@mellanox.com,
	brouer@redhat.com, borkmann@iogearbox.net, tariqt@mellanox.com,
	john.fastabend@gmail.com, jakub.kicinski@netronome.com,
	netdev@vger.kernel.org, saeedm@dev.mellanox.co.il
In-Reply-To: <1530920986.7664.5.camel@intel.com>

On Fri, Jul 06, 2018 at 11:49:55PM +0000, Waskiewicz Jr, Peter wrote:
> On Fri, 2018-07-06 at 16:38 -0700, Alexei Starovoitov wrote:
> > On Fri, Jul 06, 2018 at 08:44:24PM +0000, Waskiewicz Jr, Peter wrote:
> > > On Fri, 2018-07-06 at 09:30 -0700, Alexei Starovoitov wrote:
> > > > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:
> > > > > 
> > > > > I'm also not 100% on board with the argument that "future" FW
> > > > > can
> > > > > reshuffle things whatever way it wants to.  Is the assumption
> > > > > that
> > > > > future ASICs/FW will be designed to always use the "blessed"
> > > > > BTF
> > > > > format?  Or will it be reconfigurable at runtime?
> > > > 
> > > > let's table configuration of metadata aside for a second.
> > > 
> > > I agree that this should/could be NIC-specific and shouldn't weigh
> > > on
> > > the metadata interface between the drivers and XDP layer.
> > > 
> > > > Describing metedata layout in BTF allows NICs to disclose
> > > > everything
> > > > NIC has to users in a standard and generic way.
> > > > Whether firmware is reconfigurable on the fly or has to reflashed
> > > > and hw powercycled to have new md layout (and corresponding BTF
> > > > description)
> > > > is a separate discussion.
> > > > Saeed's proposal introduces the concept of 'offset' inside
> > > > 'struct
> > > > xdp_md_info'
> > > > to reach 'hash' value in metadata.
> > > > Essentially it's a run-time way to access 'hash' instead of
> > > > build-
> > > > time.
> > > > So bpf program would need two loads to read csum or hash field
> > > > instead of one.
> > > > With BTF the layout of metadata is known to the program at build-
> > > > time.
> > > > 
> > > > To reiterate the proposal:
> > > > - driver+firmware keep layout of the metadata in BTF format
> > > > (either
> > > > in the driver
> > > >   or driver can read it from firmware)
> > > > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will
> > > > query
> > > > the driver and
> > > >   generate normal C header file based on BTF in the given NIC
> > > > - user does #include "md_desc.h" and bpf program can access md-
> > > > >csum
> > > > or md->hash
> > > >   with direct single load out of metadata area in front of the
> > > > packet
> > > 
> > > This piece is where I'd like to discuss more.  When we discussed
> > > this
> > > in Seoul, the initial proposal was a static struct that we'd try to
> > > hammer out a common layout between the interested parties.  That
> > > obviously wasn't going to scale, and we wanted to pursue something
> > > more
> > > dynamic.  But I thought the goal was the XDP/eBPF program wouldn't
> > > want
> > > to care what the underlying device is, and could just ask for
> > > metadata
> > > that it's interested in.  With this approach, your eBPF program is
> > > now
> > > bound/tied to the NIC/driver, and if you switched to a differen
> > > NIC/driver combo, then you'd have to rewrite part of your eBPF
> > > program
> > > to comprehend that.  I thought we were trying to avoid that.
> > 
> > It looks to me that NICs have a lot more differences instead of
> > common pieces.
> > If we focus the architecture on making common things as generic
> > as possible we miss the bigger picture of covering distinct
> > and unique features that hw provides.
> > 
> > In the last email when I said "would be good to standardize at least
> > a few common fields"
> > I meant that it make sense only at the later phases.
> > I don't think we should put things into uapi upfront.
> > Otherwise we will end up with likely useless fields.
> > Like vlan field. Surely a lot of nics can store it into metadata, but
> > why?
> > bpf program can easily read it from the packet.
> > What's the benefit of adding it to md?
> > This metadata concept we're discussing in the context of program
> > acceleration.
> > If metadata doesn't give perf benefits it should not be done by
> > firmware, it should not be supported by the driver and certainly
> > should not be part of uapi.
> > Hence I'd like to see pure BTF description without any
> > common/standard
> > fields across the drivers and figure out what (if anything) is useful
> > to accelerate xdp programs (and af_xdp in the future).
> 
> I guess I didn't write my response very well before, apologies.  I
> actually really like the BTF description and the dynamic-ness of it. 
> It solves lots of problems and keeps things out of the UAPI.
> 
> What I was getting at was using the BTF description and dumping a
> header file to be used in the program to be loaded.  If we have an
> application developer include that header, it will be the Intel BTF
> description, or Mellanox BTF description, etc.  I'm fine with that, but
> it makes your program not as portable, since you'd need to get a
> different header and recompile it if you change the NIC.  Unless I'm
> missing something.

Correct. It makes programs not portable.
That is phase one to figure out what's useful.
Rewriting of offsets and figuring out which fields should be
standardized in uapi is a second phase to make those programs
actually portable.

Once it becomes obvious that feature X of vendor Y is useful
to accelerate particular well known production xdp application
I suspect other vendors will find a way to tweak their firmware
to provide the same feature. Right now vendor <-> customer discussions
are somewhat baseless. Vendors believe that what they have in firmware
is useful, but sw owners may be don't see it this way and don't have
a way to try it and cannot prove one way or the other.
This will help us get unstuck: expose features that nics have
while not baking anything into uapi

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Alexei Starovoitov @ 2018-07-07  0:53 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Daniel Borkmann, Saeed Mahameed, saeedm@dev.mellanox.co.il,
	alexander.h.duyck@intel.com, netdev@vger.kernel.org, Tariq Toukan,
	john.fastabend@gmail.com, brouer@redhat.com,
	borkmann@iogearbox.net, peter.waskiewicz.jr@intel.com
In-Reply-To: <20180706170847.3b2c7809@cakuba.netronome.com>

On Fri, Jul 06, 2018 at 05:08:47PM -0700, Jakub Kicinski wrote:
> On Fri, 6 Jul 2018 16:42:51 -0700, Alexei Starovoitov wrote:
> > On Fri, Jul 06, 2018 at 02:33:58PM -0700, Jakub Kicinski wrote:
> > > On Fri, 6 Jul 2018 09:30:42 -0700, Alexei Starovoitov wrote:  
> > > > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:  
> > > > > 
> > > > > I'm also not 100% on board with the argument that "future" FW can
> > > > > reshuffle things whatever way it wants to.  Is the assumption that
> > > > > future ASICs/FW will be designed to always use the "blessed" BTF
> > > > > format?  Or will it be reconfigurable at runtime?    
> > > > 
> > > > let's table configuration of metadata aside for a second.
> > > > 
> > > > Describing metedata layout in BTF allows NICs to disclose everything
> > > > NIC has to users in a standard and generic way.
> > > > Whether firmware is reconfigurable on the fly or has to reflashed
> > > > and hw powercycled to have new md layout (and corresponding BTF description)
> > > > is a separate discussion.
> > > > Saeed's proposal introduces the concept of 'offset' inside 'struct xdp_md_info'
> > > > to reach 'hash' value in metadata.
> > > > Essentially it's a run-time way to access 'hash' instead of build-time.
> > > > So bpf program would need two loads to read csum or hash field instead of one.
> > > > With BTF the layout of metadata is known to the program at build-time.
> > > > 
> > > > To reiterate the proposal:
> > > > - driver+firmware keep layout of the metadata in BTF format (either in the driver
> > > >   or driver can read it from firmware)
> > > > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will query the driver and
> > > >   generate normal C header file based on BTF in the given NIC
> > > > - user does #include "md_desc.h" and bpf program can access md->csum or md->hash
> > > >   with direct single load out of metadata area in front of the packet
> > > > - llvm compiles bpf program and records how program is doing this md->csum accesses
> > > >   in BTF format as well (the compiler will be keeping such records
> > > >   for __sk_buff and all other structs too, but that's separate discussion)
> > > > - during sys_bpf(prog_load) the kernel checks (via supplied BTF) that the way the program
> > > >   accesses metadata (and other structs) matches BTF from the driver,
> > > >   so no surprises if driver+firmware got updated, but program is not recompiled
> > > > - every NIC can have their own layout of metadata and its own meaning of the fields,
> > > >   but would be good to standardize at least a few common fields like hash  
> > > 
> > > Can I expose HW descriptors this way, though, or is the proposal to
> > > copy this data into the packet buffer?  
> > 
> > That crossed my mind too. We can use BTF to describe HW descriptors too,
> > but I don't think it would buy us anything. AF_XDP approach is better.
> >
> > > > Once this is working we can do more cool things with BTF.
> > > > Like doing offset rewriting at program load time similar to what we plan
> > > > to do for tracing. Tracing programs will be doing 'task->pid' access
> > > > and the kernel will adjust offsetof(struct task_struct, pid) during program load
> > > > depending on BTF for the kernel.
> > > > The same trick we can do for networking metadata.
> > > > The program will contain load instruction md->hash that will get automatically
> > > > adjusted to proper offset depending on BTF of 'hash' field in the NIC.
> > > > For now I'm proposing _not_ to go that far with offset rewriting and start
> > > > with simple steps described above.  
> > > 
> > > Why? :(  Could we please go with the rewrite/driver helpers instead of
> > > impacting fast paths of the drivers yet again?  This rewrite should be
> > > easier than task->pid, because we have the synthetic user space struct
> > > xdp_md definition.  
> > 
> > I don't understand 'impact fast path yet again' concern.
> > If NIC has certain metadata today, just derscribe what it has in BTF
> > and supply the actual per-packet md to xdp program as-is.
> > No changes for fast path at all.
> > Future rewritting will be done by the bpf/xdp core with zero
> > changes to the driver. All driver has to do is to provide BTF.
> 
> I'm confused.  AFAIK most *existing* NICs have the metadata in the
> "descriptor", i.e. not in the packet buffer.  So if the NIC just
> describes what it has, and there is no data shuffling/copying
> (performance) then we have to expose the descriptor, no?

which piece of sw put that data into desciptor ?
I bet it's firmware. It could have stored it into pre-packet data, no?
I'd like to avoid _all_ copies.
Right now xdp program can only see a pointer to packet and pre-packet.
If we need another pointer to a piece of the packet descriptor,
that's also fine. Both pre-packet metadata and pieces of descriptor
can be described in BTF.
I'd like to push back on firmware folks that should be listening
to feedback from driver folks and kernel stack instead of saying
'here is hw spec that firmware provides'. Firmware is software.
It can change and should be open to change by the community
with proper maintainership.

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Alexei Starovoitov @ 2018-07-07  1:00 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Waskiewicz Jr, Peter, Duyck, Alexander H, daniel@iogearbox.net,
	saeedm@mellanox.com, brouer@redhat.com, borkmann@iogearbox.net,
	tariqt@mellanox.com, john.fastabend@gmail.com,
	netdev@vger.kernel.org, saeedm@dev.mellanox.co.il
In-Reply-To: <20180706174043.14a8145a@cakuba.netronome.com>

On Fri, Jul 06, 2018 at 05:40:43PM -0700, Jakub Kicinski wrote:
> 
> Could we just say that at the start we expose only existing SKB fields
> (csum, hash, mark) and the meaning of the is the same as in the SKB?

what would be the meaning of 'hash' ? Over which fields?
Does it support inner and outer packets? How about udp encap (vxlan and friends) ?
Same question of csum... tcp only? how about crc for sctp?
What is 'mark' ?

^ permalink raw reply

* KASAN: stack-out-of-bounds Read in vmalloc_fault
From: syzbot @ 2018-07-07  1:19 UTC (permalink / raw)
  To: bridge, coreteam, davem, fw, kadlec, linux-kernel, netdev,
	netfilter-devel, pablo, stephen, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    6fcf9b1d4d6c r8169: fix runtime suspend
git tree:       bpf-next
console output: https://syzkaller.appspot.com/x/log.txt?x=174529b2400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=d264f2b04177ca7c
dashboard link: https://syzkaller.appspot.com/bug?extid=7b269953d076326d7de0
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=1397e2c2400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=120e462c400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+7b269953d076326d7de0@syzkaller.appspotmail.com

IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
IPv6: ADDRCONF(NETDEV_UP): team0: link is not ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KASAN: stack-out-of-bounds in pgd_val  
arch/x86/include/asm/paravirt.h:412 [inline]
BUG: KASAN: stack-out-of-bounds in p4d_pfn  
arch/x86/include/asm/pgtable.h:205 [inline]
BUG: KASAN: stack-out-of-bounds in vmalloc_fault+0x743/0x760  
arch/x86/mm/fault.c:462
Read of size 8 at addr ffff8801a936f160 by task ksoftirqd/0/9

CPU: 0 PID: 9 Comm: ksoftirqd/0 Not tainted 4.18.0-rc3+ #47
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
  pgd_val arch/x86/include/asm/paravirt.h:412 [inline]
  p4d_pfn arch/x86/include/asm/pgtable.h:205 [inline]
  vmalloc_fault+0x743/0x760 arch/x86/mm/fault.c:462
  __do_page_fault+0x829/0xe50 arch/x86/mm/fault.c:1245
  do_page_fault+0xf6/0x8c0 arch/x86/mm/fault.c:1471
  page_fault+0x1e/0x30 arch/x86/entry/entry_64.S:1160
RIP: 0010:ebt_do_table+0x1a7/0x2170 net/bridge/netfilter/ebtables.c:208
Code: 24 04 4d 8d ac 24 80 00 00 00 48 89 fa 48 c1 ea 03 0f b6 14 02 48 89  
f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 8f 1e 00 00 <41> 8b 44 24 04  
49 8d 7c 24 38 65 8b 15 28 7f 39 79 48 c1 e0 04 48
RSP: 0018:ffff8801d9ab6ab8 EFLAGS: 00010246
RAX: 0000000000000007 RBX: 0000000000000005 RCX: ffffffff81601dff
RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffffc90001e2c004
RBP: ffff8801d9ab6c88 R08: ffffed003a6f4e49 R09: ffffed003a6f4e48
R10: ffffed003a6f4e48 R11: ffff8801d37a7243 R12: ffffc90001e2c000
R13: ffffc90001e2c080 R14: ffff8801d9ab6cc0 R15: ffff8801d9394380
  ebt_broute+0x200/0x340 net/bridge/netfilter/ebtable_broute.c:60
  br_handle_frame+0x6d1/0x1a20 net/bridge/br_input.c:292
  __netif_receive_skb_core.constprop.140+0x142d/0x3620 net/core/dev.c:4697
  __netif_receive_skb_one_core+0xd0/0x200 net/core/dev.c:4766
  __netif_receive_skb+0x2c/0x1e0 net/core/dev.c:4873
  process_backlog+0x219/0x760 net/core/dev.c:5676
  napi_poll net/core/dev.c:6088 [inline]
  net_rx_action+0x7a5/0x1950 net/core/dev.c:6154
  __do_softirq+0x2e8/0xb17 kernel/softirq.c:288
  run_ksoftirqd+0x86/0x100 kernel/softirq.c:649
  smpboot_thread_fn+0x417/0x870 kernel/smpboot.c:164
  kthread+0x345/0x410 kernel/kthread.c:240
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412

The buggy address belongs to the page:
page:ffffea0006a4dbc0 count:1 mapcount:0 mapping:0000000000000000 index:0x0
flags: 0x2fffc0000000000()
raw: 02fffc0000000000 dead000000000100 dead000000000200 0000000000000000
raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8801a936f000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff8801a936f080: 00 00 00 f1 f1 f1 f1 00 f2 f2 f2 f2 f2 f2 f2 00
> ffff8801a936f100: f2 f2 f2 f2 f2 f2 f2 00 f2 f2 f2 f2 f2 f2 f2 f8
                                                        ^
  ffff8801a936f180: f2 f2 f2 f2 f2 f2 f2 00 f2 f2 f2 00 00 00 00 00
  ffff8801a936f200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Jakub Kicinski @ 2018-07-07  1:20 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Waskiewicz Jr, Peter, Duyck, Alexander H, daniel@iogearbox.net,
	saeedm@mellanox.com, brouer@redhat.com, borkmann@iogearbox.net,
	tariqt@mellanox.com, john.fastabend@gmail.com,
	netdev@vger.kernel.org, saeedm@dev.mellanox.co.il
In-Reply-To: <20180707010012.vzn455zmba4ocsqd@ast-mbp.dhcp.thefacebook.com>

On Fri, 6 Jul 2018 18:00:13 -0700, Alexei Starovoitov wrote:
> On Fri, Jul 06, 2018 at 05:40:43PM -0700, Jakub Kicinski wrote:
> > 
> > Could we just say that at the start we expose only existing SKB fields
> > (csum, hash, mark) and the meaning of the is the same as in the SKB?  
> 
> what would be the meaning of 'hash' ? Over which fields?
> Does it support inner and outer packets? How about udp encap (vxlan and friends) ?

We don't seem to need to answer that for the rest of the stack, no?  We
can expose the "hash type" field as well if that's *really* necessary.

> Same question of csum... tcp only? 

Shouldn't we just stick to CHECKSUM_COMPLETE?

> how about crc for sctp?

That's harder to answer.  Can cls_bpf access such info?

> What is 'mark' ?

Same thing it would be on an skb.  Most likely set with an offloaded TC
rule?

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: David Miller @ 2018-07-07  1:27 UTC (permalink / raw)
  To: alexei.starovoitov
  Cc: jakub.kicinski, daniel, saeedm, saeedm, alexander.h.duyck, netdev,
	tariqt, john.fastabend, brouer, borkmann, peter.waskiewicz.jr
In-Reply-To: <20180706163041.xstyfednmgho23m3@ast-mbp.dhcp.thefacebook.com>

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Fri, 6 Jul 2018 09:30:42 -0700

> Like doing offset rewriting at program load time similar to what we plan
> to do for tracing. Tracing programs will be doing 'task->pid' access
> and the kernel will adjust offsetof(struct task_struct, pid) during program load
> depending on BTF for the kernel.
> The same trick we can do for networking metadata.
> The program will contain load instruction md->hash that will get automatically
> adjusted to proper offset depending on BTF of 'hash' field in the NIC.
> For now I'm proposing _not_ to go that far with offset rewriting and start
> with simple steps described above.

+1

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: David Miller @ 2018-07-07  1:37 UTC (permalink / raw)
  To: alexei.starovoitov
  Cc: jakub.kicinski, daniel, saeedm, saeedm, alexander.h.duyck, netdev,
	tariqt, john.fastabend, brouer, borkmann, peter.waskiewicz.jr
In-Reply-To: <20180707005317.y255ymd7m5bw3zof@ast-mbp.dhcp.thefacebook.com>

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Fri, 6 Jul 2018 17:53:18 -0700

> I'd like to push back on firmware folks that should be listening
> to feedback from driver folks and kernel stack instead of saying
> 'here is hw spec that firmware provides'. Firmware is software.
> It can change and should be open to change by the community
> with proper maintainership.

+1

^ permalink raw reply

* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Jakub Kicinski @ 2018-07-07  1:44 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Saeed Mahameed, saeedm@dev.mellanox.co.il,
	alexander.h.duyck@intel.com, netdev@vger.kernel.org, Tariq Toukan,
	john.fastabend@gmail.com, brouer@redhat.com,
	borkmann@iogearbox.net, peter.waskiewicz.jr@intel.com
In-Reply-To: <20180707005317.y255ymd7m5bw3zof@ast-mbp.dhcp.thefacebook.com>

On Fri, 6 Jul 2018 17:53:18 -0700, Alexei Starovoitov wrote:
> On Fri, Jul 06, 2018 at 05:08:47PM -0700, Jakub Kicinski wrote:
> > On Fri, 6 Jul 2018 16:42:51 -0700, Alexei Starovoitov wrote:  
> > > On Fri, Jul 06, 2018 at 02:33:58PM -0700, Jakub Kicinski wrote:  
> > > > On Fri, 6 Jul 2018 09:30:42 -0700, Alexei Starovoitov wrote:    
> > > > > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:    
> > > > > > 
> > > > > > I'm also not 100% on board with the argument that "future" FW can
> > > > > > reshuffle things whatever way it wants to.  Is the assumption that
> > > > > > future ASICs/FW will be designed to always use the "blessed" BTF
> > > > > > format?  Or will it be reconfigurable at runtime?      
> > > > > 
> > > > > let's table configuration of metadata aside for a second.
> > > > > 
> > > > > Describing metedata layout in BTF allows NICs to disclose everything
> > > > > NIC has to users in a standard and generic way.
> > > > > Whether firmware is reconfigurable on the fly or has to reflashed
> > > > > and hw powercycled to have new md layout (and corresponding BTF description)
> > > > > is a separate discussion.
> > > > > Saeed's proposal introduces the concept of 'offset' inside 'struct xdp_md_info'
> > > > > to reach 'hash' value in metadata.
> > > > > Essentially it's a run-time way to access 'hash' instead of build-time.
> > > > > So bpf program would need two loads to read csum or hash field instead of one.
> > > > > With BTF the layout of metadata is known to the program at build-time.
> > > > > 
> > > > > To reiterate the proposal:
> > > > > - driver+firmware keep layout of the metadata in BTF format (either in the driver
> > > > >   or driver can read it from firmware)
> > > > > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will query the driver and
> > > > >   generate normal C header file based on BTF in the given NIC
> > > > > - user does #include "md_desc.h" and bpf program can access md->csum or md->hash
> > > > >   with direct single load out of metadata area in front of the packet
> > > > > - llvm compiles bpf program and records how program is doing this md->csum accesses
> > > > >   in BTF format as well (the compiler will be keeping such records
> > > > >   for __sk_buff and all other structs too, but that's separate discussion)
> > > > > - during sys_bpf(prog_load) the kernel checks (via supplied BTF) that the way the program
> > > > >   accesses metadata (and other structs) matches BTF from the driver,
> > > > >   so no surprises if driver+firmware got updated, but program is not recompiled
> > > > > - every NIC can have their own layout of metadata and its own meaning of the fields,
> > > > >   but would be good to standardize at least a few common fields like hash    
> > > > 
> > > > Can I expose HW descriptors this way, though, or is the proposal to
> > > > copy this data into the packet buffer?    
> > > 
> > > That crossed my mind too. We can use BTF to describe HW descriptors too,
> > > but I don't think it would buy us anything. AF_XDP approach is better.
> > >  
> > > > > Once this is working we can do more cool things with BTF.
> > > > > Like doing offset rewriting at program load time similar to what we plan
> > > > > to do for tracing. Tracing programs will be doing 'task->pid' access
> > > > > and the kernel will adjust offsetof(struct task_struct, pid) during program load
> > > > > depending on BTF for the kernel.
> > > > > The same trick we can do for networking metadata.
> > > > > The program will contain load instruction md->hash that will get automatically
> > > > > adjusted to proper offset depending on BTF of 'hash' field in the NIC.
> > > > > For now I'm proposing _not_ to go that far with offset rewriting and start
> > > > > with simple steps described above.    
> > > > 
> > > > Why? :(  Could we please go with the rewrite/driver helpers instead of
> > > > impacting fast paths of the drivers yet again?  This rewrite should be
> > > > easier than task->pid, because we have the synthetic user space struct
> > > > xdp_md definition.    
> > > 
> > > I don't understand 'impact fast path yet again' concern.
> > > If NIC has certain metadata today, just derscribe what it has in BTF
> > > and supply the actual per-packet md to xdp program as-is.
> > > No changes for fast path at all.
> > > Future rewritting will be done by the bpf/xdp core with zero
> > > changes to the driver. All driver has to do is to provide BTF.  
> > 
> > I'm confused.  AFAIK most *existing* NICs have the metadata in the
> > "descriptor", i.e. not in the packet buffer.  So if the NIC just
> > describes what it has, and there is no data shuffling/copying
> > (performance) then we have to expose the descriptor, no?  
> 
> which piece of sw put that data into desciptor ?
> I bet it's firmware. It could have stored it into pre-packet data, no?
> I'd like to avoid _all_ copies.
> Right now xdp program can only see a pointer to packet and pre-packet.
> If we need another pointer to a piece of the packet descriptor,
> that's also fine. Both pre-packet metadata and pieces of descriptor
> can be described in BTF.

Okay, if we expose another pointer then it's possible to avoid copies.

But please keep in mind that descriptors are very compact, there is
a lot of interdependencies between fields and the fields can shift
depending on the type of packet.  HW/FW guys always quote the 64B
packet performance as a reason why things can't be simple.  We can't
consume 50% of PCIe bandwidth to DMA the metadata alone.

> I'd like to push back on firmware folks that should be listening
> to feedback from driver folks and kernel stack instead of saying
> 'here is hw spec that firmware provides'. Firmware is software.
> It can change and should be open to change by the community
> with proper maintainership.

I think the way to influence FW/HW is to provide a strong and well
justified standard, and if we just expose raw HW data we are doing 
just the opposite.  We can claim BTF driver provides is not uABI, but
I will personally no longer feel comfortable with modifying descriptor
formats (and I've done that for the NFP in the past - grep for
chained_metadata_format).

^ permalink raw reply

* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
From: David Miller @ 2018-07-07  1:47 UTC (permalink / raw)
  To: vladimir_zapolskiy
  Cc: sergei.shtylyov, andrew, geert, netdev, linux-renesas-soc
In-Reply-To: <20180704081245.7395-1-vladimir_zapolskiy@mentor.com>

From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Date: Wed, 4 Jul 2018 11:12:38 +0300

> For ages trivial changes to RAVB and SuperH ethernet links by means of
> standard 'ethtool' trigger a 'sleeping function called from invalid
> context' bug, to visualize it on r8a7795 ULCB:
 ...
> The root cause is that an attempt to modify ECMR and GECMR registers
> only when RX/TX function is disabled was too overcomplicated in its
> original implementation, also processing of an optional Link Change
> interrupt added even more complexity, as a result the implementation
> was error prone.
> 
> The new locking scheme is confirmed to be correct by dumping driver
> specific and generic PHY framework function calls with aid of ftrace
> while running more or less advanced tests.
> 
> Please note that sh_eth patches from the series were built-tested only.
> 
> On purpose I do not add Fixes tags, the reused PHY handlers were added
> way later than the fixed problems were firstly found in the drivers.
> 
> Changes from v1 to v2:
> * the original patches are split to bugfixes and enhancements only,
>   both v1 and v2 series are absolutely equal in total, thus I omit
>   description of changes in individual patches,
> * the latter implies that there should be no strict need for retesting,
>   but because formally two series are different, I have to drop the tags
>   given by Geert and Andrew, please send your tags again.

Series applied with Fixes: tags of patches 8 and 9 fixed up.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/3] fix use-after-free bugs in skb list processing
From: David Miller @ 2018-07-07  1:56 UTC (permalink / raw)
  To: ecree; +Cc: netdev
In-Reply-To: <3c007026-e27a-6ac9-4a29-9746840e24ed@solarflare.com>

From: Edward Cree <ecree@solarflare.com>
Date: Fri, 6 Jul 2018 18:01:04 +0100

> A couple of bugs in skb list handling were spotted by Dan Carpenter, with
>  the help of Smatch; following up on them I found a couple more similar
>  cases.  This series fixes them by changing the relevant loops to use the
>  dequeue-enqueue model (rather than in-place list modification), and then
>  adds a list.h helper macro to refactor code using the dequeue-enqueue
>  model.

Edward, I think the helper added in patch #3 is not all that great.

It can be even more confusing to understand what the code is doing in
my opinion.  I am pretty sure more hackers would need to go and
read the macro definition in list.h before understanding what the
read side effects of it are.

Would you please resubmit this series including only patches #1
and #2?

Thank you.

^ permalink raw reply

* Re: [PATCH net-next 0/6] sock cookie initializers
From: David Miller @ 2018-07-07  1:59 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <20180706141259.29295-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Fri,  6 Jul 2018 10:12:53 -0400

> From: Willem de Bruijn <willemb@google.com>
> 
> Recent UDP GSO and SO_TXTIME features added new fields to cookie
> structs.
> 
> When adding a field, all sites where a struct is initialized have to
> be updated, which is a lot of boilerplate. Alternatively, a field can
> be initialized selectively, but this is fragile. I introduced a bug
> in udp gso where an uninitialized field was read. See also fix commit
> ("9887cba19978 ip: limit use of gso_size to udp").
> 
> Introduce initializers for structs ipcm(6)_cookie and sockc_cookie.
> 
> patch 1..3 do exactly this.
> patch 4..5 make ipv4 and ipv6 handle cookies the same way and
>            remove some boilerplate in doing so.
> patch 6    removes the udp gso branch that needed the above fix

Series applied, thanks Willem.

^ permalink raw reply

* Re: [PATCH net-next 3/6] net: mvneta: discriminate error cause for missed packet
From: David Miller @ 2018-07-07  2:09 UTC (permalink / raw)
  To: gregory.clement
  Cc: linux-kernel, netdev, thomas.petazzoni, linux-arm-kernel, jason,
	andrew, sebastian.hesselbarth, yelena, nadavh, mw, dima,
	antoine.tenart, miquel.raynal, maxime.chevallier
In-Reply-To: <20180706131949.2684-4-gregory.clement@bootlin.com>

From: Gregory CLEMENT <gregory.clement@bootlin.com>
Date: Fri,  6 Jul 2018 15:19:46 +0200

> @@ -609,6 +606,10 @@ struct mvneta_rx_queue {
>  
>  	/* Index of the next RX DMA descriptor to process */
>  	int next_desc_to_proc;
> +
> +	/* error counters */
> +	u32 skb_alloc_err;
> +	u32 refill_err;
>  };

These counters aren't exported anywhere, making them not so useful.

Either remove this stuff or add them to the driver's ethtool state.

Thank you.

^ permalink raw reply

* Re: [PATCH V2 net-next] liquidio: fix kernel panic when NIC firmware is older than 1.7.2
From: David Miller @ 2018-07-07  2:10 UTC (permalink / raw)
  To: felix.manlunas
  Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
	ricardo.farrington
In-Reply-To: <20180706182707.GA1720@felix-thinkpad.cavium.com>

From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Fri, 6 Jul 2018 11:27:07 -0700

> From: Rick Farrington <ricardo.farrington@cavium.com>
> 
> Pre-1.7.2 NIC firmware does not support (and does not respond to) the "get
> speed" command which is sent by the 1.7.2 driver (for CN23XX-225 cards
> only) during modprobe.  Due to a bug in older firmware (with respect to
> unknown commands), this unsupported command causes a cascade of errors that
> ends in a kernel panic.
> 
> Fix it by making the sending of the "get speed" command conditional on the
> firmware version.
> 
> Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next 00/10] Misc. bug fixes & cleanups for HNS3 driver
From: David Miller @ 2018-07-07  2:13 UTC (permalink / raw)
  To: salil.mehta
  Cc: yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
	linuxarm
In-Reply-To: <20180706102804.196-1-salil.mehta@huawei.com>

From: Salil Mehta <salil.mehta@huawei.com>
Date: Fri, 6 Jul 2018 11:27:54 +0100

> This patch-set presents some miscellaneous bug fixes and cleanups
> for the HNS3 driver.

Series applied, thank you.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox