* Re: [RFC v4 0/3] vhost: introduce mdev based hardware backend
From: Tiwei Bie @ 2019-09-20 2:16 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S. Tsirkin, alex.williamson, maxime.coquelin,
linux-kernel, kvm, virtualization, netdev, dan.daly,
cunming.liang, zhihong.wang, lingshan.zhu
In-Reply-To: <43aaf7dc-f08b-8898-3c55-908ff4d68866@redhat.com>
On Fri, Sep 20, 2019 at 09:30:58AM +0800, Jason Wang wrote:
> On 2019/9/19 下午11:45, Tiwei Bie wrote:
> > On Thu, Sep 19, 2019 at 09:08:11PM +0800, Jason Wang wrote:
> > > On 2019/9/18 下午10:32, Michael S. Tsirkin wrote:
> > > > > > > So I have some questions:
> > > > > > >
> > > > > > > 1) Compared to method 2, what's the advantage of creating a new vhost char
> > > > > > > device? I guess it's for keep the API compatibility?
> > > > > > One benefit is that we can avoid doing vhost ioctls on
> > > > > > VFIO device fd.
> > > > > Yes, but any benefit from doing this?
> > > > It does seem a bit more modular, but it's certainly not a big deal.
> > > Ok, if we go this way, it could be as simple as provide some callback to
> > > vhost, then vhost can just forward the ioctl through parent_ops.
> > >
> > > > > > > 2) For method 2, is there any easy way for user/admin to distinguish e.g
> > > > > > > ordinary vfio-mdev for vhost from ordinary vfio-mdev?
> > > > > > I think device-api could be a choice.
> > > > > Ok.
> > > > >
> > > > >
> > > > > > > I saw you introduce
> > > > > > > ops matching helper but it's not friendly to management.
> > > > > > The ops matching helper is just to check whether a given
> > > > > > vfio-device is based on a mdev device.
> > > > > >
> > > > > > > 3) A drawback of 1) and 2) is that it must follow vfio_device_ops that
> > > > > > > assumes the parameter comes from userspace, it prevents support kernel
> > > > > > > virtio drivers.
> > > > > > >
> > > > > > > 4) So comes the idea of method 3, since it register a new vhost-mdev driver,
> > > > > > > we can use device specific ops instead of VFIO ones, then we can have a
> > > > > > > common API between vDPA parent and vhost-mdev/virtio-mdev drivers.
> > > > > > As the above draft shows, this requires introducing a new
> > > > > > VFIO device driver. I think Alex's opinion matters here.
> > > Just to clarify, a new type of mdev driver but provides dummy
> > > vfio_device_ops for VFIO to make container DMA ioctl work.
> > I see. Thanks! IIUC, you mean we can provide a very tiny
> > VFIO device driver in drivers/vhost/mdev.c, e.g.:
> >
> > static int vfio_vhost_mdev_open(void *device_data)
> > {
> > if (!try_module_get(THIS_MODULE))
> > return -ENODEV;
> > return 0;
> > }
> >
> > static void vfio_vhost_mdev_release(void *device_data)
> > {
> > module_put(THIS_MODULE);
> > }
> >
> > static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = {
> > .name = "vfio-vhost-mdev",
> > .open = vfio_vhost_mdev_open,
> > .release = vfio_vhost_mdev_release,
> > };
> >
> > static int vhost_mdev_probe(struct device *dev)
> > {
> > struct mdev_device *mdev = to_mdev_device(dev);
> >
> > ... Check the mdev device_id proposed in ...
> > ... https://lkml.org/lkml/2019/9/12/151 ...
>
>
> To clarify, this should be done through the id_table fields in
> vhost_mdev_driver, and it should claim it supports virtio-mdev device only:
>
>
> static struct mdev_class_id id_table[] = {
> { MDEV_ID_VIRTIO },
> { 0 },
> };
>
>
> static struct mdev_driver vhost_mdev_driver = {
> ...
> .id_table = id_table,
> }
In this way, both of virtio-mdev and vhost-mdev will try to
take this device. We may want a way to let vhost-mdev take this
device only when users explicitly ask it to do it. Or maybe we
can have a different MDEV_ID for vhost-mdev but share the device
ops with virtio-mdev.
>
>
> >
> > return vfio_add_group_dev(dev, &vfio_vhost_mdev_dev_ops, mdev);
>
>
> And in vfio_vhost_mdev_ops, all its need is to just implement vhost-net
> ioctl and translate them to virtio-mdev transport (e.g device_ops I proposed
> or ioctls other whatever other method) API.
I see, so my previous understanding is basically correct:
https://lkml.org/lkml/2019/9/17/332
I.e. we won't have a separate vhost fd and we will do all vhost
ioctls on the VFIO device fd backed by this new VFIO driver.
> And it could have a dummy ops
> implementation for the other device_ops.
>
>
> > }
> >
> > static void vhost_mdev_remove(struct device *dev)
> > {
> > vfio_del_group_dev(dev);
> > }
> >
> > static struct mdev_driver vhost_mdev_driver = {
> > .name = "vhost_mdev",
> > .probe = vhost_mdev_probe,
> > .remove = vhost_mdev_remove,
> > };
> >
> > So we can bind above mdev driver to the virtio-mdev compatible
> > mdev devices when we want to use vhost-mdev.
> >
> > After binding above driver to the mdev device, we can setup IOMMU
> > via VFIO and get VFIO device fd of this mdev device, and pass it
> > to vhost fd (/dev/vhost-mdev) with a SET_BACKEND ioctl.
>
>
> Then what vhost-mdev char device did is just forwarding ioctl back to this
> vfio device fd which seems a overkill. It's simpler that just do ioctl on
> the device ops directly.
Yes.
Thanks,
Tiwei
>
> Thanks
>
>
> >
> > Thanks,
> > Tiwei
> >
> > > Thanks
> > >
> > >
> > > > > Yes, it is.
> > > > >
> > > > > Thanks
> > > > >
> > > > >
^ permalink raw reply
* Re: [RFC v4 0/3] vhost: introduce mdev based hardware backend
From: Jason Wang @ 2019-09-20 2:36 UTC (permalink / raw)
To: Tiwei Bie
Cc: Michael S. Tsirkin, alex.williamson, maxime.coquelin,
linux-kernel, kvm, virtualization, netdev, dan.daly,
cunming.liang, zhihong.wang, lingshan.zhu
In-Reply-To: <20190920021630.GA4108@___>
On 2019/9/20 上午10:16, Tiwei Bie wrote:
> On Fri, Sep 20, 2019 at 09:30:58AM +0800, Jason Wang wrote:
>> On 2019/9/19 下午11:45, Tiwei Bie wrote:
>>> On Thu, Sep 19, 2019 at 09:08:11PM +0800, Jason Wang wrote:
>>>> On 2019/9/18 下午10:32, Michael S. Tsirkin wrote:
>>>>>>>> So I have some questions:
>>>>>>>>
>>>>>>>> 1) Compared to method 2, what's the advantage of creating a new vhost char
>>>>>>>> device? I guess it's for keep the API compatibility?
>>>>>>> One benefit is that we can avoid doing vhost ioctls on
>>>>>>> VFIO device fd.
>>>>>> Yes, but any benefit from doing this?
>>>>> It does seem a bit more modular, but it's certainly not a big deal.
>>>> Ok, if we go this way, it could be as simple as provide some callback to
>>>> vhost, then vhost can just forward the ioctl through parent_ops.
>>>>
>>>>>>>> 2) For method 2, is there any easy way for user/admin to distinguish e.g
>>>>>>>> ordinary vfio-mdev for vhost from ordinary vfio-mdev?
>>>>>>> I think device-api could be a choice.
>>>>>> Ok.
>>>>>>
>>>>>>
>>>>>>>> I saw you introduce
>>>>>>>> ops matching helper but it's not friendly to management.
>>>>>>> The ops matching helper is just to check whether a given
>>>>>>> vfio-device is based on a mdev device.
>>>>>>>
>>>>>>>> 3) A drawback of 1) and 2) is that it must follow vfio_device_ops that
>>>>>>>> assumes the parameter comes from userspace, it prevents support kernel
>>>>>>>> virtio drivers.
>>>>>>>>
>>>>>>>> 4) So comes the idea of method 3, since it register a new vhost-mdev driver,
>>>>>>>> we can use device specific ops instead of VFIO ones, then we can have a
>>>>>>>> common API between vDPA parent and vhost-mdev/virtio-mdev drivers.
>>>>>>> As the above draft shows, this requires introducing a new
>>>>>>> VFIO device driver. I think Alex's opinion matters here.
>>>> Just to clarify, a new type of mdev driver but provides dummy
>>>> vfio_device_ops for VFIO to make container DMA ioctl work.
>>> I see. Thanks! IIUC, you mean we can provide a very tiny
>>> VFIO device driver in drivers/vhost/mdev.c, e.g.:
>>>
>>> static int vfio_vhost_mdev_open(void *device_data)
>>> {
>>> if (!try_module_get(THIS_MODULE))
>>> return -ENODEV;
>>> return 0;
>>> }
>>>
>>> static void vfio_vhost_mdev_release(void *device_data)
>>> {
>>> module_put(THIS_MODULE);
>>> }
>>>
>>> static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = {
>>> .name = "vfio-vhost-mdev",
>>> .open = vfio_vhost_mdev_open,
>>> .release = vfio_vhost_mdev_release,
>>> };
>>>
>>> static int vhost_mdev_probe(struct device *dev)
>>> {
>>> struct mdev_device *mdev = to_mdev_device(dev);
>>>
>>> ... Check the mdev device_id proposed in ...
>>> ... https://lkml.org/lkml/2019/9/12/151 ...
>>
>> To clarify, this should be done through the id_table fields in
>> vhost_mdev_driver, and it should claim it supports virtio-mdev device only:
>>
>>
>> static struct mdev_class_id id_table[] = {
>> { MDEV_ID_VIRTIO },
>> { 0 },
>> };
>>
>>
>> static struct mdev_driver vhost_mdev_driver = {
>> ...
>> .id_table = id_table,
>> }
> In this way, both of virtio-mdev and vhost-mdev will try to
> take this device. We may want a way to let vhost-mdev take this
> device only when users explicitly ask it to do it. Or maybe we
> can have a different MDEV_ID for vhost-mdev but share the device
> ops with virtio-mdev.
I think it's similar to virtio-pci vs vfio-pci. User can choose to
switch the driver through bind/unbind.
>
>>
>>> return vfio_add_group_dev(dev, &vfio_vhost_mdev_dev_ops, mdev);
>>
>> And in vfio_vhost_mdev_ops, all its need is to just implement vhost-net
>> ioctl and translate them to virtio-mdev transport (e.g device_ops I proposed
>> or ioctls other whatever other method) API.
> I see, so my previous understanding is basically correct:
>
> https://lkml.org/lkml/2019/9/17/332
>
> I.e. we won't have a separate vhost fd and we will do all vhost
> ioctls on the VFIO device fd backed by this new VFIO driver.
Yes.
Thanks
>
>> And it could have a dummy ops
>> implementation for the other device_ops.
>>
>>
>>> }
>>>
>>> static void vhost_mdev_remove(struct device *dev)
>>> {
>>> vfio_del_group_dev(dev);
>>> }
>>>
>>> static struct mdev_driver vhost_mdev_driver = {
>>> .name = "vhost_mdev",
>>> .probe = vhost_mdev_probe,
>>> .remove = vhost_mdev_remove,
>>> };
>>>
>>> So we can bind above mdev driver to the virtio-mdev compatible
>>> mdev devices when we want to use vhost-mdev.
>>>
>>> After binding above driver to the mdev device, we can setup IOMMU
>>> via VFIO and get VFIO device fd of this mdev device, and pass it
>>> to vhost fd (/dev/vhost-mdev) with a SET_BACKEND ioctl.
>>
>> Then what vhost-mdev char device did is just forwarding ioctl back to this
>> vfio device fd which seems a overkill. It's simpler that just do ioctl on
>> the device ops directly.
> Yes.
>
> Thanks,
> Tiwei
>
>
>> Thanks
>>
>>
>>> Thanks,
>>> Tiwei
>>>
>>>> Thanks
>>>>
>>>>
>>>>>> Yes, it is.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>>
^ permalink raw reply
* [PATCH] can: gs_usb: prevent memory leak
From: Navid Emamdoost @ 2019-09-20 2:44 UTC (permalink / raw)
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Wolfgang Grandegger,
Marc Kleine-Budde, David S. Miller, Greg Kroah-Hartman,
Alexios Zavras, Allison Randal, Thomas Gleixner, linux-can,
netdev, linux-kernel
In gs_can_open if usb_submit_urb fails the allocated urb should be
released.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/can/usb/gs_usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index bd6eb9967630..2f74f6704c12 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -623,6 +623,7 @@ static int gs_can_open(struct net_device *netdev)
rc);
usb_unanchor_urb(urb);
+ usb_free_urb(urb);
break;
}
--
2.17.1
^ permalink raw reply related
* Re: [RFC {net,iproute2}-next 0/2] Introduce an eBPF hookpoint for tx queue selection in the XPS (Transmit Packet Steering) code.
From: Matt Cover @ 2019-09-20 2:45 UTC (permalink / raw)
To: Jason Wang
Cc: davem, ast, daniel, kafai, songliubraving, yhs, nikolay, sd,
sbrivio, vincent, kda, Matthew Cover, jiri, Eric Dumazet, pabeni,
idosch, petrm, f.fainelli, stephen, dsahern, christian,
jakub.kicinski, Roopa Prabhu, johannes.berg, mkubecek, netdev,
linux-kernel, bpf
In-Reply-To: <fa17e10b-d61b-00c3-b3eb-02cf6a197b78@redhat.com>
On Thu, Sep 19, 2019 at 6:42 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2019/9/20 上午8:05, Matt Cover wrote:
> > On Thu, Sep 19, 2019 at 3:45 PM Matthew Cover <werekraken@gmail.com> wrote:
> >> WORK IN PROGRESS:
> >> * bpf program loading works!
> >> * txq steering via bpf program return code works!
> >> * bpf program unloading not working.
> >> * bpf program attached query not working.
> >>
> >> This patch set provides a bpf hookpoint with goals similar to, but a more
> >> generic implementation than, TUNSETSTEERINGEBPF; userspace supplied tx queue
> >> selection policy.
>
>
> One point that I introduce TUNSETSTEERINGEBPF instead of using a generic
> way like cls/act bpf is that I need make sure to have a consistent API
> with macvtap.
>
> In the case of macvtap, TX means transmit from userspace to kernel, but
> for TUN, it means transmit from kernel to userspace.
>
Ah, ok. I'll have to check that out at some point.
>
> >>
> >> TUNSETSTEERINGEBPF is a useful bpf hookpoint, but has some drawbacks.
> >>
> >> First, it only works on tun/tap devices.
> >>
> >> Second, there is no way in the current TUNSETSTEERINGEBPF implementation
> >> to bail out or load a noop bpf prog and fallback to the no prog tx queue
> >> selection method.
>
>
> I believe it expect that eBPF should take all the parts (even the
> fallback part).
>
This would be easy to change in the existing TUNSETSTEERINGEBPF
implementation if desired. We'd just need a negative return from the bpf prog
to result in falling back to tun_automq_select_queue(). If that behavior
sounds reasonable to you, I can look into that as a separate patch.
>
> >>
> >> Third, the TUNSETSTEERINGEBPF interface seems to require possession of existing
> >> or creation of new queues/fds.
>
>
> That's the way TUN work for past +10 years because ioctl is the only way
> to do configuration and it requires a fd to carry that. David suggest to
> implement netlink but nobody did that.
>
I see.
>
> >>
> >> This most naturally fits in the "wire" implementation since possession of fds
> >> is ensured. However, it also means the various "wire" implementations (e.g.
> >> qemu) have to all be made aware of TUNSETSTEERINGEBPF and expose an interface
> >> to load/unload a bpf prog (or provide a mechanism to pass an fd to another
> >> program).
>
>
> The load/unload of ebpf program is standard bpf() syscall. Ioctl just
> attach that to TUN. This idea is borrowed from packet socket which the
> bpf program was attached through setsockopt().
>
Yeah, it doesn't take much code to load a prog. I wrote one earlier this week
in fact which spins up an extra fd and detaches right after.
>
> >>
> >> Alternatively, you can spin up an extra queue and immediately disable via
> >> IFF_DETACH_QUEUE, but this seems unsafe; packets could be enqueued to this
> >> extra file descriptor which is part of our bpf prog loader, not our "wire".
>
>
> You can use you 'wire' queue to do ioctl, but we can invent other API.
>
It might be cool to provide a way to create an already detached fd
(not sure if this
is non-trivial for some reason). Switching over to netlink could be
the more long
term goal.
>
> >>
> >> Placing this in the XPS code and leveraging iproute2 and rtnetlink to provide
> >> our bpf prog loader in a similar manner to xdp gives us a nice way to separate
> >> the tap "wire" and the loading of tx queue selection policy. It also lets us
> >> use this hookpoint for any device traversing XPS.
> >>
> >> This patch only introduces the new hookpoint to the XPS code and will not yet
> >> be used by tun/tap devices using the intree tun.ko (which implements an
> >> .ndo_select_queue and does not traverse the XPS code).
> >>
> >> In a future patch set, we can optionally refactor tun.ko to traverse this call
> >> to bpf_prog_run_clear_cb() and bpf prog storage. tun/tap devices could then
> >> leverage iproute2 as a generic loader. The TUNSETSTEERINGEBPF interface could
> >> at this point be optionally deprecated/removed.
>
>
> As described above, we need it for macvtap and you propose here can not
> work for that.
>
> I'm not against this proposal, just want to clarify some considerations
> when developing TUNSETSTEERINGEPF. The main goal is for VM to implement
> sophisticated steering policy like RSS without touching kernel.
>
Very cool. Thank you for your comments Jason; they have added clarity
to some things.
I'm still interested in adding this hookpoint, community willing. I
believe it provides
value beyond xps_cpus/xps_rxqs.
I also plan to look into adding a similar hookpoint in the rps code.
That will unlock
additional possibilities for this xps hookpoint (e.g. rfs implemented
via bpf maps, but
only on a subset of traffic [high priority or especially resource
costly] rather than all).
I've had (so far casual) chats with a couple NIC vendors about various
"SmartNICs" supporting custom entropy fields for RSS. I'm playing with the idea
of an "rpsoffload" prog loaded into the NIC being the way custom entropy is
configured. Being able to configure RSS to generate a hash based on an fields
of an inner packet or a packet type specific field like GRE key would be super
nice for NFV workloads.
Perhaps even an "rpsdrv" or "rpsoffload" hookpoint could leverage bpf
helpers for
RSS hash algorithm (e.g. bfp_rss_hash_toeplitz(), bpf_rss_hash_crc(),
bpf_rss_hash_xor(), etc.).
The ideas on how things would look for receive are still early, but I
think there is
a lot of potential for making things more flexible by leveraging ebpf
in this area.
> Thanks
>
>
> >>
> >> Both patches in this set have been tested using a rebuilt tun.ko with no
> >> .ndo_select_queue.
> >>
> >> sed -i '/\.ndo_select_queue.*=/d' drivers/net/tun.c
> >>
> >> The tap device was instantiated using tap_mq_pong.c, supporting scripts, and
> >> wrapping service found here:
> >>
> >> https://github.com/stackpath/rxtxcpu/tree/v1.2.6/helpers
> >>
> >> The bpf prog source and test scripts can be found here:
> >>
> >> https://github.com/werekraken/xps_ebpf
> >>
> >> In nstxq, netsniff-ng using PACKET_FANOUT_QM is leveraged to check the
> >> queue_mapping.
> >>
> >> With no prog loaded, the tx queue selection is adhering our xps_cpus
> >> configuration.
> >>
> >> [vagrant@localhost ~]$ grep . /sys/class/net/tap0/queues/tx-*/xps_cpus; ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe;
> >> /sys/class/net/tap0/queues/tx-0/xps_cpus:1
> >> /sys/class/net/tap0/queues/tx-1/xps_cpus:2
> >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.146 ms
> >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.121 ms
> >> cpu1: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >>
> >> With a return 0 bpg prog, our tx queue is 0 (despite xps_cpus).
> >>
> >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello0.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.160 ms
> >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.124 ms
> >> cpu1: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> ping-4852 [000] .... 2691.633260: 0: xps (RET 0): Hello, World!
> >> ping-4869 [001] .... 2695.753588: 0: xps (RET 0): Hello, World!
> >>
> >> With a return 1 bpg prog, our tx queue is 1.
> >>
> >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello1.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.193 ms
> >> cpu0: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.135 ms
> >> cpu1: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> ping-4894 [000] .... 2710.652080: 0: xps (RET 1): Hello, World!
> >> ping-4911 [001] .... 2714.774608: 0: xps (RET 1): Hello, World!
> >>
> >> With a return 2 bpg prog, our tx queue is 0 (we only have 2 tx queues).
> >>
> >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello2.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=1.20 ms
> >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.986 ms
> >> cpu1: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> ping-4936 [000] .... 2729.442668: 0: xps (RET 2): Hello, World!
> >> ping-4953 [001] .... 2733.614558: 0: xps (RET 2): Hello, World!
> >>
> >> With a return -1 bpf prog, our tx queue selection is once again determined by
> >> xps_cpus. Any negative return should work the same and provides a nice
> >> mechanism to bail out or have a noop bpf prog at this hookpoint.
> >>
> >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello_neg1.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.628 ms
> >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.322 ms
> >> cpu1: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> >> ping-4981 [000] .... 2763.510760: 0: xps (RET -1): Hello, World!
> >> ping-4998 [001] .... 2767.632583: 0: xps (RET -1): Hello, World!
> >>
> >> bpf prog unloading is not yet working and neither does `ip link show` report
> >> when an "xps" bpf prog is attached. This is my first time touching iproute2 or
> >> rtnetlink, so it may be something obvious to those more familiar.
> > Adding Jason... sorry for missing that the first time.
^ permalink raw reply
* [PATCH] rtl8xxxu: prevent leaking urb
From: Navid Emamdoost @ 2019-09-20 3:00 UTC (permalink / raw)
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Jes Sorensen,
Kalle Valo, David S. Miller, linux-wireless, netdev, linux-kernel
In rtl8xxxu_submit_int_urb if usb_submit_urb fails the allocated urb
should be released.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 8136e268b4e6..4a559c37e208 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5443,6 +5443,7 @@ static int rtl8xxxu_submit_int_urb(struct ieee80211_hw *hw)
ret = usb_submit_urb(urb, GFP_KERNEL);
if (ret) {
usb_unanchor_urb(urb);
+ usb_free_urb(urb);
goto error;
}
--
2.17.1
^ permalink raw reply related
* Re: [v3 2/3] samples: pktgen: add helper functions for IP(v4/v6) CIDR parsing
From: Daniel T. Lee @ 2019-09-20 3:04 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Jesper Dangaard Brouer, David S . Miller, netdev
In-Reply-To: <87sgowl7xe.fsf@toke.dk>
Thanks for the feedback!
How about just capturing with "Result: OK" except for 'pgctrl'?
(more details are in the diff below)
~/git/linux/net$ ag -Q 'Result:'
core/pktgen.c
702: seq_printf(seq, "Result: %s\n", pkt_dev->result);
704: seq_puts(seq, "Result: Idle\n");
1739: seq_printf(seq, "\nResult: %s\n", t->result);
1741: seq_puts(seq, "\nResult: NA\n");
The upper command shows that 'Result: ' string is only printed on
'pktgen_if' and 'pktgen_thread'. So I thought just changing to below diff
will solve the problem.
diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
index 87ae61701904..38cf9f62502f 100644
--- a/samples/pktgen/functions.sh
+++ b/samples/pktgen/functions.sh
@@ -60,6 +60,7 @@ function pg_set() {
function proc_cmd() {
local result
local proc_file=$1
+ local status=0
# after shift, the remaining args are contained in $@
shift
local proc_ctrl=${PROC_DIR}/$proc_file
@@ -75,13 +76,14 @@ function proc_cmd() {
echo "cmd: $@ > $proc_ctrl"
fi
# Quoting of "$@" is important for space expansion
- echo "$@" > "$proc_ctrl"
- local status=$?
+ echo "$@" > "$proc_ctrl" || status=$?
- result=$(grep "Result: OK:" $proc_ctrl)
- # Due to pgctrl, cannot use exit code $? from grep
- if [[ "$result" == "" ]]; then
- grep "Result:" $proc_ctrl >&2
+ if [[ "$proc_file" != "pgctrl" ]]; then
+ result=$(grep "Result: OK:" $proc_ctrl) || true
+ # Due to pgctrl, cannot use exit code $? from grep
+ if [[ "$result" == "" ]]; then
+ grep "Result:" $proc_ctrl >&2
+ fi
fi
> We actually want to continue, and output what command that failed (and
> also grep again after "Result:" to provide the kernel reason).
Since, with 'true', the command won't fail and continue, and on error with
'pktgen_if' and 'pktgen_thread', It'll grep again after "Result:" to provide
the kernel reason. [1][2]
> I'd argue that fixing this is the right thing to do... Maybe add set -o
> nounset as well while we're at it? :)
Adding nounset option could break working with $IP6. Most of the scripts
tries to check if $IP6 variable exists whether it is defined or not.
> Even if you somehow "fix" function proc_cmd, then we in general want to
> catch different error situations by looking at status $?, and output
> meaning full errors via calling err() function. IHMO as minimum with
> errexit you need a 'trap' function that can help/inform the user of
> what went wrong.
I agree. trap ERR will help with more sophisticated handling of errors, but
I'm not sure which to elaborate more (about what went wrong) compared
to current error format? (which is grep again after "Result:" to provide the
kernel reason.)
I really appreciate your time and effort for the review.
Thanks,
Daniel
[1]: pktgen_if_show:
https://elixir.bootlin.com/linux/latest/source/net/core/pktgen.c#L702
[2]: pktgen_thread_show:
https://elixir.bootlin.com/linux/latest/source/net/core/pktgen.c#L1739
^ permalink raw reply related
* Re: [RFC v4 3/3] vhost: introduce mdev based hardware backend
From: Tiwei Bie @ 2019-09-20 4:21 UTC (permalink / raw)
To: Jason Wang
Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <0b0f74ba-8958-dd7d-3e98-f7a58b1ec5f7@redhat.com>
On Tue, Sep 17, 2019 at 03:26:30PM +0800, Jason Wang wrote:
> On 2019/9/17 上午9:02, Tiwei Bie wrote:
> > diff --git a/drivers/vhost/mdev.c b/drivers/vhost/mdev.c
> > new file mode 100644
> > index 000000000000..8c6597aff45e
> > --- /dev/null
> > +++ b/drivers/vhost/mdev.c
> > @@ -0,0 +1,462 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018-2019 Intel Corporation.
> > + */
> > +
> > +#include <linux/compat.h>
> > +#include <linux/kernel.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/mdev.h>
> > +#include <linux/module.h>
> > +#include <linux/vfio.h>
> > +#include <linux/vhost.h>
> > +#include <linux/virtio_mdev.h>
> > +
> > +#include "vhost.h"
> > +
> > +struct vhost_mdev {
> > + struct mutex mutex;
> > + struct vhost_dev dev;
> > + struct vhost_virtqueue *vqs;
> > + int nvqs;
> > + u64 state;
> > + u64 features;
> > + u64 acked_features;
> > + struct vfio_group *vfio_group;
> > + struct vfio_device *vfio_device;
> > + struct mdev_device *mdev;
> > +};
> > +
> > +/*
> > + * XXX
> > + * We assume virtio_mdev.ko exposes below symbols for now, as we
> > + * don't have a proper way to access parent ops directly yet.
> > + *
> > + * virtio_mdev_readl()
> > + * virtio_mdev_writel()
> > + */
> > +extern u32 virtio_mdev_readl(struct mdev_device *mdev, loff_t off);
> > +extern void virtio_mdev_writel(struct mdev_device *mdev, loff_t off, u32 val);
>
>
> Need to consider a better approach, I feel we should do it through some kind
> of mdev driver instead of talk to mdev device directly.
Yeah, a better approach is really needed here.
Besides, we may want a way to allow accessing the mdev
device_ops proposed in below series outside the
drivers/vfio/mdev/ directory.
https://lkml.org/lkml/2019/9/12/151
I.e. allow putting mdev drivers outside above directory.
> > +
> > + for (queue_id = 0; queue_id < m->nvqs; queue_id++) {
> > + vq = &m->vqs[queue_id];
> > +
> > + if (!vq->desc || !vq->avail || !vq->used)
> > + break;
> > +
> > + virtio_mdev_writel(mdev, VIRTIO_MDEV_QUEUE_NUM, vq->num);
> > +
> > + if (!vhost_translate_ring_addr(vq, (u64)vq->desc,
> > + vhost_get_desc_size(vq, vq->num),
> > + &addr))
> > + return -EINVAL;
>
>
> Interesting, any reason for doing such kinds of translation to HVA? I
> believe the add should already an IOVA that has been map by VFIO.
Currently, in the software based vhost-kernel and vhost-user
backends, QEMU will pass ring addresses as HVA in SET_VRING_ADDR
ioctl when iotlb isn't enabled. If it's OK to let QEMU pass GPA
in vhost-mdev in this case, then this translation won't be needed.
Thanks,
Tiwei
^ permalink raw reply
* [PATCH RFC 3/5] net: Add a netdev software feature set that defaults to off.
From: Steffen Klassert @ 2019-09-20 4:49 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, Willem de Bruijn, Paolo Abeni,
Subash Abhinov Kasiviswanathan, Marcelo Ricardo Leitner
In-Reply-To: <20190920044905.31759-1-steffen.klassert@secunet.com>
The previous patch added the NETIF_F_GRO_FRAGLIST feature.
This is a software feature that should default to off.
Current software features default to on, so add a new
feature set that defaults to off.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/linux/netdev_features.h | 3 +++
net/core/dev.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index b239507da2a0..34d050bb1ae6 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -230,6 +230,9 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
/* changeable features with no special hardware requirements */
#define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO)
+/* Changeable features with no special hardware requirements that defaults to off. */
+#define NETIF_F_SOFT_FEATURES_OFF NETIF_F_GRO_FRAGLIST
+
#define NETIF_F_VLAN_FEATURES (NETIF_F_HW_VLAN_CTAG_FILTER | \
NETIF_F_HW_VLAN_CTAG_RX | \
NETIF_F_HW_VLAN_CTAG_TX | \
diff --git a/net/core/dev.c b/net/core/dev.c
index b1afafee3e2a..cc0bbec0f1d7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8730,7 +8730,7 @@ int register_netdevice(struct net_device *dev)
/* Transfer changeable features to wanted_features and enable
* software offloads (GSO and GRO).
*/
- dev->hw_features |= NETIF_F_SOFT_FEATURES;
+ dev->hw_features |= (NETIF_F_SOFT_FEATURES | NETIF_F_SOFT_FEATURES_OFF);
dev->features |= NETIF_F_SOFT_FEATURES;
if (dev->netdev_ops->ndo_udp_tunnel_add) {
--
2.17.1
^ permalink raw reply related
* [PATCH RFC 4/5] net: Support GRO/GSO fraglist chaining.
From: Steffen Klassert @ 2019-09-20 4:49 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, Willem de Bruijn, Paolo Abeni,
Subash Abhinov Kasiviswanathan, Marcelo Ricardo Leitner
In-Reply-To: <20190920044905.31759-1-steffen.klassert@secunet.com>
This patch adds the core functions to chain/unchain
GSO skbs at the frag_list pointer. This also adds
a new GSO type SKB_GSO_FRAGLIST and a is_flist
flag to napi_gro_cb which indicates that this
flow will be GROed by fraglist chaining.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/linux/netdevice.h | 4 +-
include/linux/skbuff.h | 2 +
net/core/dev.c | 2 +-
net/core/skbuff.c | 106 ++++++++++++++++++++++++++++++++++++++
4 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4917cf513bd1..d037e31a1acb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2306,7 +2306,8 @@ struct napi_gro_cb {
/* Number of gro_receive callbacks this packet already went through */
u8 recursion_counter:4;
- /* 1 bit hole */
+ /* GRO is done by frag_list pointer chaining. */
+ u8 is_flist:1;
/* used to support CHECKSUM_COMPLETE for tunneling protocols */
__wsum csum;
@@ -2656,6 +2657,7 @@ struct net_device *dev_get_by_napi_id(unsigned int napi_id);
int netdev_get_name(struct net *net, char *name, int ifindex);
int dev_restart(struct net_device *dev);
int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb);
+int skb_gro_receive_list(struct sk_buff *p, struct sk_buff *skb);
static inline unsigned int skb_gro_offset(const struct sk_buff *skb)
{
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c72540813ea7..3d5fd0a0eea7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3511,6 +3511,8 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet);
bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu);
bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len);
struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
+struct sk_buff *skb_segment_list(struct sk_buff *skb, netdev_features_t features,
+ unsigned int offset);
struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
int skb_ensure_writable(struct sk_buff *skb, int write_len);
int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci);
diff --git a/net/core/dev.c b/net/core/dev.c
index cc0bbec0f1d7..f2a66198154d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3109,7 +3109,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
segs = skb_mac_gso_segment(skb, features);
- if (unlikely(skb_needs_check(skb, tx_path) && !IS_ERR(segs)))
+ if (segs != skb && unlikely(skb_needs_check(skb, tx_path) && !IS_ERR(segs)))
skb_warn_bad_offload(skb);
return segs;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 2b40b5a9425b..3ff56677a6fb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3638,6 +3638,112 @@ static inline skb_frag_t skb_head_frag_to_page_desc(struct sk_buff *frag_skb)
return head_frag;
}
+struct sk_buff *skb_segment_list(struct sk_buff *skb,
+ netdev_features_t features,
+ unsigned int offset)
+{
+ struct sk_buff *list_skb = skb_shinfo(skb)->frag_list;
+ unsigned int tnl_hlen = skb_tnl_header_len(skb);
+ unsigned int delta_truesize = 0;
+ unsigned int delta_len = 0;
+ struct sk_buff *tail = NULL;
+ struct sk_buff *nskb;
+
+ skb_push(skb, -skb_network_offset(skb) + offset);
+
+ skb_shinfo(skb)->frag_list = NULL;
+
+ do {
+ nskb = list_skb;
+ list_skb = list_skb->next;
+
+ if (!tail)
+ skb->next = nskb;
+ else
+ tail->next = nskb;
+
+ tail = nskb;
+
+ delta_len += nskb->len;
+ delta_truesize += nskb->truesize;
+
+ skb_push(nskb, -skb_network_offset(nskb) + offset);
+
+ if (!secpath_exists(nskb))
+ __skb_ext_copy(nskb, skb);
+
+ memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
+
+ nskb->ip_summed = skb->ip_summed;
+ nskb->csum_valid = skb->csum_valid;
+ nskb->tstamp = skb->tstamp;
+ nskb->dev = skb->dev;
+ nskb->queue_mapping = skb->queue_mapping;
+
+ nskb->mac_len = skb->mac_len;
+ nskb->mac_header = skb->mac_header;
+ nskb->transport_header = skb->transport_header;
+ nskb->network_header = skb->network_header;
+ skb_dst_copy(nskb, skb);
+
+ skb_headers_offset_update(nskb, skb_headroom(nskb) - skb_headroom(skb));
+ skb_copy_from_linear_data_offset(skb, -tnl_hlen,
+ nskb->data - tnl_hlen,
+ offset + tnl_hlen);
+
+ if (skb_needs_linearize(nskb, features) &&
+ __skb_linearize(nskb))
+ goto err_linearize;
+
+ } while (list_skb);
+
+ skb->truesize = skb->truesize - delta_truesize;
+ skb->data_len = skb->data_len - delta_len;
+ skb->len = skb->len - delta_len;
+
+ skb_gso_reset(skb);
+
+ skb->prev = tail;
+
+ if (skb_needs_linearize(skb, features) &&
+ __skb_linearize(skb))
+ goto err_linearize;
+
+ skb_get(skb);
+
+ return skb;
+
+err_linearize:
+ kfree_skb_list(skb->next);
+ skb->next = NULL;
+ return ERR_PTR(-ENOMEM);
+}
+EXPORT_SYMBOL_GPL(skb_segment_list);
+
+int skb_gro_receive_list(struct sk_buff *p, struct sk_buff *skb)
+{
+ if (unlikely(p->len + skb->len >= 65536))
+ return -E2BIG;
+
+ if (NAPI_GRO_CB(p)->last == p)
+ skb_shinfo(p)->frag_list = skb;
+ else
+ NAPI_GRO_CB(p)->last->next = skb;
+
+ skb_pull(skb, skb_gro_offset(skb));
+
+ NAPI_GRO_CB(p)->last = skb;
+ NAPI_GRO_CB(p)->count++;
+ p->data_len += skb->len;
+ p->truesize += skb->truesize;
+ p->len += skb->len;
+
+ NAPI_GRO_CB(skb)->same_flow = 1;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(skb_gro_receive_list);
+
/**
* skb_segment - Perform protocol segmentation on skb.
* @head_skb: buffer to segment
--
2.17.1
^ permalink raw reply related
* [PATCH RFC 1/5] UDP: enable GRO by default.
From: Steffen Klassert @ 2019-09-20 4:49 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, Willem de Bruijn, Paolo Abeni,
Subash Abhinov Kasiviswanathan, Marcelo Ricardo Leitner
In-Reply-To: <20190920044905.31759-1-steffen.klassert@secunet.com>
This patch enables UDP GRO regardless if a GRO capable
socket is present. With this GRO is done by default
for the local input and forwarding path.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/udp.h | 2 +-
net/ipv4/udp_offload.c | 38 ++++++++++++++++----------------------
net/ipv6/udp_offload.c | 10 ++++++++--
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/include/net/udp.h b/include/net/udp.h
index bad74f780831..44e0e52b585c 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -167,7 +167,7 @@ typedef struct sock *(*udp_lookup_t)(struct sk_buff *skb, __be16 sport,
__be16 dport);
struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
- struct udphdr *uh, udp_lookup_t lookup);
+ struct udphdr *uh, struct sock *sk);
int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index a3908e55ed89..929b12fc7bc5 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -401,36 +401,25 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
return NULL;
}
-INDIRECT_CALLABLE_DECLARE(struct sock *udp6_lib_lookup_skb(struct sk_buff *skb,
- __be16 sport, __be16 dport));
struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
- struct udphdr *uh, udp_lookup_t lookup)
+ struct udphdr *uh, struct sock *sk)
{
struct sk_buff *pp = NULL;
struct sk_buff *p;
struct udphdr *uh2;
unsigned int off = skb_gro_offset(skb);
int flush = 1;
- struct sock *sk;
- rcu_read_lock();
- sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb,
- udp4_lib_lookup_skb, skb, uh->source, uh->dest);
- if (!sk)
- goto out_unlock;
-
- if (udp_sk(sk)->gro_enabled) {
+ if (!sk || !udp_sk(sk)->gro_receive) {
pp = call_gro_receive(udp_gro_receive_segment, head, skb);
- rcu_read_unlock();
return pp;
}
if (NAPI_GRO_CB(skb)->encap_mark ||
(skb->ip_summed != CHECKSUM_PARTIAL &&
NAPI_GRO_CB(skb)->csum_cnt == 0 &&
- !NAPI_GRO_CB(skb)->csum_valid) ||
- !udp_sk(sk)->gro_receive)
- goto out_unlock;
+ !NAPI_GRO_CB(skb)->csum_valid))
+ goto out;
/* mark that this skb passed once through the tunnel gro layer */
NAPI_GRO_CB(skb)->encap_mark = 1;
@@ -457,8 +446,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
-out_unlock:
- rcu_read_unlock();
+out:
skb_gro_flush_final(skb, pp, flush);
return pp;
}
@@ -468,8 +456,10 @@ INDIRECT_CALLABLE_SCOPE
struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb)
{
struct udphdr *uh = udp_gro_udphdr(skb);
+ struct sk_buff *pp;
+ struct sock *sk;
- if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
+ if (unlikely(!uh))
goto flush;
/* Don't bother verifying checksum if we're going to flush anyway. */
@@ -484,7 +474,11 @@ struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb)
inet_gro_compute_pseudo);
skip:
NAPI_GRO_CB(skb)->is_ipv6 = 0;
- return udp_gro_receive(head, skb, uh, udp4_lib_lookup_skb);
+ rcu_read_lock();
+ sk = static_branch_unlikely(&udp_encap_needed_key) ? udp4_lib_lookup_skb(skb, uh->source, uh->dest) : NULL;
+ pp = udp_gro_receive(head, skb, uh, sk);
+ rcu_read_unlock();
+ return pp;
flush:
NAPI_GRO_CB(skb)->flush = 1;
@@ -517,9 +511,7 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
rcu_read_lock();
sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb,
udp4_lib_lookup_skb, skb, uh->source, uh->dest);
- if (sk && udp_sk(sk)->gro_enabled) {
- err = udp_gro_complete_segment(skb);
- } else if (sk && udp_sk(sk)->gro_complete) {
+ if (sk && udp_sk(sk)->gro_complete) {
skb_shinfo(skb)->gso_type = uh->check ? SKB_GSO_UDP_TUNNEL_CSUM
: SKB_GSO_UDP_TUNNEL;
@@ -529,6 +521,8 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
skb->encapsulation = 1;
err = udp_sk(sk)->gro_complete(sk, skb,
nhoff + sizeof(struct udphdr));
+ } else {
+ err = udp_gro_complete_segment(skb);
}
rcu_read_unlock();
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 64b8f05d6735..435cfbadb6bd 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -115,8 +115,10 @@ INDIRECT_CALLABLE_SCOPE
struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb)
{
struct udphdr *uh = udp_gro_udphdr(skb);
+ struct sk_buff *pp;
+ struct sock *sk;
- if (unlikely(!uh) || !static_branch_unlikely(&udpv6_encap_needed_key))
+ if (unlikely(!uh))
goto flush;
/* Don't bother verifying checksum if we're going to flush anyway. */
@@ -132,7 +134,11 @@ struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb)
skip:
NAPI_GRO_CB(skb)->is_ipv6 = 1;
- return udp_gro_receive(head, skb, uh, udp6_lib_lookup_skb);
+ rcu_read_lock();
+ sk = static_branch_unlikely(&udpv6_encap_needed_key) ? udp6_lib_lookup_skb(skb, uh->source, uh->dest) : NULL;
+ pp = udp_gro_receive(head, skb, uh, sk);
+ rcu_read_unlock();
+ return pp;
flush:
NAPI_GRO_CB(skb)->flush = 1;
--
2.17.1
^ permalink raw reply related
* [PATCH RFC 0/5] Support fraglist GRO/GSO
From: Steffen Klassert @ 2019-09-20 4:49 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, Willem de Bruijn, Paolo Abeni,
Subash Abhinov Kasiviswanathan, Marcelo Ricardo Leitner
This patchset adds support to do GRO/GSO by chaining packets
of the same flow at the SKB frag_list pointer. This avoids
the overhead to merge payloads into one big packet, and
on the other end, if GSO is needed it avoids the overhead
of splitting the big packet back to the native form.
Patch 1 Enables UDP GRO by default.
Patch 2 adds netdev feature flags to enable listifyed GRO,
this implements one of the configuration options discussed
at netconf 2019.
Patch 3 adds a netdev software feature set that defaults to off
and assigns the new listifyed GRO feature flag to it.
Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
GRO supported socket is found.
I have only meaningful forwarding performance measurements.
I did some tests for the local receive path with netperf and iperf,
but in this case the sender that generates the packets is the
bottleneck. So the benchmarks are not that meaningful for the
receive path.
Paolo Abeni did some benchmarks of the local receive path for the v2
version of this pachset, results can be found here:
https://www.spinics.net/lists/netdev/msg551158.html
I used my IPsec forwarding test setup for the performance measurements:
------------ ------------
-->| router 1 |-------->| router 2 |--
| ------------ ------------ |
| |
| -------------------- |
--------|Spirent Testcenter|<----------
--------------------
net-next (September 7th):
Single stream UDP frame size 1460 Bytes: 1.161.000 fps (13.5 Gbps).
----------------------------------------------------------------------
net-next (September 7th) + standard UDP GRO/GSO:
Single stream UDP frame size 1460 Bytes: 1.801.000 fps (21 Gbps).
----------------------------------------------------------------------
net-next (September 7th) + fraglist UDP GRO/GSO:
Single stream UDP frame size 1460 Bytes: 2.860.000 fps (33.4 Gbps).
-----------------------------------------------------------------------
Changes from v1:
- Add IPv6 support.
- Split patchset to enable UDP GRO by default before adding
fraglist GRO support.
- Mark fraglist GRO packets as CHECKSUM_NONE.
- Take a refcount on the first segment skb when doing fraglist
segmentation. With this we can use the same error handling
path as with standard segmentation.
Changes from v2:
- Add a netdev feature flag to configure listifyed GRO.
- Fix UDP GRO enabling for IPv6.
- Fix a rcu_read_lock() imbalance.
- Fix error path in skb_segment_list().
Changes from v3:
- Rename NETIF_F_GRO_LIST to NETIF_F_GRO_FRAGLIST and add
NETIF_F_GSO_FRAGLIST.
- Move introduction of SKB_GSO_FRAGLIST to patch 2.
- Use udpv6_encap_needed_key instead of udp_encap_needed_key in IPv6.
- Move some missplaced code from patch 5 to patch 1 where it belongs to.
^ permalink raw reply
* [PATCH RFC 2/5] net: Add fraglist GRO/GSO feature flags
From: Steffen Klassert @ 2019-09-20 4:49 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, Willem de Bruijn, Paolo Abeni,
Subash Abhinov Kasiviswanathan, Marcelo Ricardo Leitner
In-Reply-To: <20190920044905.31759-1-steffen.klassert@secunet.com>
This adds new Fraglist GRO/GSO feature flags. They will be used
to configure fraglist GRO/GSO what will be implemented with some
followup paches.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/linux/netdev_features.h | 6 +++++-
include/linux/netdevice.h | 1 +
include/linux/skbuff.h | 2 ++
net/core/ethtool.c | 1 +
4 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 4b19c544c59a..b239507da2a0 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -53,8 +53,9 @@ enum {
NETIF_F_GSO_ESP_BIT, /* ... ESP with TSO */
NETIF_F_GSO_UDP_BIT, /* ... UFO, deprecated except tuntap */
NETIF_F_GSO_UDP_L4_BIT, /* ... UDP payload GSO (not UFO) */
+ NETIF_F_GSO_FRAGLIST_BIT, /* ... Fraglist GSO */
/**/NETIF_F_GSO_LAST = /* last bit, see GSO_MASK */
- NETIF_F_GSO_UDP_L4_BIT,
+ NETIF_F_GSO_FRAGLIST_BIT,
NETIF_F_FCOE_CRC_BIT, /* FCoE CRC32 */
NETIF_F_SCTP_CRC_BIT, /* SCTP checksum offload */
@@ -80,6 +81,7 @@ enum {
NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
+ NETIF_F_GRO_FRAGLIST_BIT, /* Fraglist GRO */
/*
* Add your fresh new feature above and remember to update
@@ -150,6 +152,8 @@ enum {
#define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
#define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
#define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
+#define NETIF_F_GRO_FRAGLIST __NETIF_F(GRO_FRAGLIST)
+#define NETIF_F_GSO_FRAGLIST __NETIF_F(GSO_FRAGLIST)
/* Finds the next feature with the highest number of the range of start till 0.
*/
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d7d5626002e9..4917cf513bd1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4524,6 +4524,7 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
BUILD_BUG_ON(SKB_GSO_ESP != (NETIF_F_GSO_ESP >> NETIF_F_GSO_SHIFT));
BUILD_BUG_ON(SKB_GSO_UDP != (NETIF_F_GSO_UDP >> NETIF_F_GSO_SHIFT));
BUILD_BUG_ON(SKB_GSO_UDP_L4 != (NETIF_F_GSO_UDP_L4 >> NETIF_F_GSO_SHIFT));
+ BUILD_BUG_ON(SKB_GSO_FRAGLIST != (NETIF_F_GSO_FRAGLIST >> NETIF_F_GSO_SHIFT));
return (features & feature) == feature;
}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 028e684fa974..c72540813ea7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -595,6 +595,8 @@ enum {
SKB_GSO_UDP = 1 << 16,
SKB_GSO_UDP_L4 = 1 << 17,
+
+ SKB_GSO_FRAGLIST = 1 << 18,
};
#if BITS_PER_LONG > 32
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6288e69e94fc..2eaf94debbf6 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
[NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
[NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
[NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
+ [NETIF_F_GRO_FRAGLIST_BIT] = "rx-gro-list",
};
static const char
--
2.17.1
^ permalink raw reply related
* [PATCH RFC 5/5] udp: Support UDP fraglist GRO/GSO.
From: Steffen Klassert @ 2019-09-20 4:49 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, Willem de Bruijn, Paolo Abeni,
Subash Abhinov Kasiviswanathan, Marcelo Ricardo Leitner
In-Reply-To: <20190920044905.31759-1-steffen.klassert@secunet.com>
This patch extends UDP GRO to support fraglist GRO/GSO
by using the previously introduced infrastructure.
All UDP packets that are not targeted to a GRO capable
UDP sockets are going to fraglist GRO now (local input
and forward).
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/udp_offload.c | 56 +++++++++++++++++++++++++++++++++++++++---
net/ipv6/udp_offload.c | 9 +++++++
2 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 929b12fc7bc5..37daafb06d4c 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -184,6 +184,20 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
}
EXPORT_SYMBOL(skb_udp_tunnel_segment);
+static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
+ netdev_features_t features)
+{
+ unsigned int mss = skb_shinfo(skb)->gso_size;
+
+ skb = skb_segment_list(skb, features, skb_mac_header_len(skb));
+ if (IS_ERR(skb))
+ return skb;
+
+ udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
+
+ return skb;
+}
+
struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
netdev_features_t features)
{
@@ -196,6 +210,9 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
__sum16 check;
__be16 newlen;
+ if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST)
+ return __udp_gso_segment_list(gso_skb, features);
+
mss = skb_shinfo(gso_skb)->gso_size;
if (gso_skb->len <= sizeof(*uh) + mss)
return ERR_PTR(-EINVAL);
@@ -354,6 +371,7 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
struct udphdr *uh2;
struct sk_buff *p;
unsigned int ulen;
+ int ret = 0;
/* requires non zero csum, for symmetry with GSO */
if (!uh->check) {
@@ -369,7 +387,6 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
}
/* pull encapsulating udp header */
skb_gro_pull(skb, sizeof(struct udphdr));
- skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
list_for_each_entry(p, head, list) {
if (!NAPI_GRO_CB(p)->same_flow)
@@ -383,14 +400,35 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
continue;
}
+ if (NAPI_GRO_CB(skb)->is_flist != NAPI_GRO_CB(p)->is_flist) {
+ NAPI_GRO_CB(skb)->flush = 1;
+ return p;
+ }
+
/* Terminate the flow on len mismatch or if it grow "too much".
* Under small packet flood GRO count could elsewhere grow a lot
* leading to excessive truesize values.
* On len mismatch merge the first packet shorter than gso_size,
* otherwise complete the GRO packet.
*/
- if (ulen > ntohs(uh2->len) || skb_gro_receive(p, skb) ||
- ulen != ntohs(uh2->len) ||
+ if (ulen > ntohs(uh2->len)) {
+ pp = p;
+ } else {
+ if (NAPI_GRO_CB(skb)->is_flist) {
+ if (!pskb_may_pull(skb, skb_gro_offset(skb))) {
+ NAPI_GRO_CB(skb)->flush = 1;
+ return NULL;
+ }
+ ret = skb_gro_receive_list(p, skb);
+ } else {
+ skb_gro_postpull_rcsum(skb, uh,
+ sizeof(struct udphdr));
+
+ ret = skb_gro_receive(p, skb);
+ }
+ }
+
+ if (ret || ulen != ntohs(uh2->len) ||
NAPI_GRO_CB(p)->count >= UDP_GRO_CNT_MAX)
pp = p;
@@ -411,6 +449,9 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
int flush = 1;
if (!sk || !udp_sk(sk)->gro_receive) {
+ if (skb->dev->features & NETIF_F_GRO_LIST)
+ NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled: 1;
+
pp = call_gro_receive(udp_gro_receive_segment, head, skb);
return pp;
}
@@ -538,6 +579,15 @@ INDIRECT_CALLABLE_SCOPE int udp4_gro_complete(struct sk_buff *skb, int nhoff)
const struct iphdr *iph = ip_hdr(skb);
struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
+ if (NAPI_GRO_CB(skb)->is_flist) {
+ uh->len = htons(skb->len - nhoff);
+
+ skb_shinfo(skb)->gso_type |= (SKB_GSO_FRAGLIST|SKB_GSO_UDP_L4);
+ skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
+
+ return 0;
+ }
+
if (uh->check)
uh->check = ~udp_v4_check(skb->len - nhoff, iph->saddr,
iph->daddr, 0);
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 435cfbadb6bd..8836f2b69ef3 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -150,6 +150,15 @@ INDIRECT_CALLABLE_SCOPE int udp6_gro_complete(struct sk_buff *skb, int nhoff)
const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
+ if (NAPI_GRO_CB(skb)->is_flist) {
+ uh->len = htons(skb->len - nhoff);
+
+ skb_shinfo(skb)->gso_type |= (SKB_GSO_FRAGLIST|SKB_GSO_UDP_L4);
+ skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
+
+ return 0;
+ }
+
if (uh->check)
uh->check = ~udp_v6_check(skb->len - nhoff, &ipv6h->saddr,
&ipv6h->daddr, 0);
--
2.17.1
^ permalink raw reply related
* [PATCH] qede: qede_fp: simplify a bit 'qede_rx_build_skb()'
From: Christophe JAILLET @ 2019-09-20 4:56 UTC (permalink / raw)
To: aelior, GR-everest-linux-l2, davem
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
Use 'skb_put_data()' instead of rewritting it.
This improves readability.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/qlogic/qede/qede_fp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 0ae28f0d2523..004c0bfec41d 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -779,8 +779,7 @@ qede_rx_build_skb(struct qede_dev *edev,
return NULL;
skb_reserve(skb, pad);
- memcpy(skb_put(skb, len),
- page_address(bd->data) + offset, len);
+ skb_put_data(skb, page_address(bd->data) + offset, len);
qede_reuse_page(rxq, bd);
goto out;
}
--
2.20.1
^ permalink raw reply related
* Re: [RFC {net,iproute2}-next 0/2] Introduce an eBPF hookpoint for tx queue selection in the XPS (Transmit Packet Steering) code.
From: Matt Cover @ 2019-09-20 5:01 UTC (permalink / raw)
To: Jason Wang
Cc: davem, ast, daniel, kafai, songliubraving, yhs, nikolay, sd,
sbrivio, vincent, kda, Matthew Cover, jiri, Eric Dumazet, pabeni,
idosch, petrm, f.fainelli, stephen, dsahern, christian,
jakub.kicinski, Roopa Prabhu, johannes.berg, mkubecek, netdev,
linux-kernel, bpf
In-Reply-To: <CAGyo_hpQSeFJYNuYvXp326C0646YUYr9CbBm-TpLobeOEaTgzg@mail.gmail.com>
On Thu, Sep 19, 2019 at 7:45 PM Matt Cover <werekraken@gmail.com> wrote:
>
> On Thu, Sep 19, 2019 at 6:42 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > On 2019/9/20 上午8:05, Matt Cover wrote:
> > > On Thu, Sep 19, 2019 at 3:45 PM Matthew Cover <werekraken@gmail.com> wrote:
> > >> WORK IN PROGRESS:
> > >> * bpf program loading works!
> > >> * txq steering via bpf program return code works!
> > >> * bpf program unloading not working.
> > >> * bpf program attached query not working.
> > >>
> > >> This patch set provides a bpf hookpoint with goals similar to, but a more
> > >> generic implementation than, TUNSETSTEERINGEBPF; userspace supplied tx queue
> > >> selection policy.
> >
> >
> > One point that I introduce TUNSETSTEERINGEBPF instead of using a generic
> > way like cls/act bpf is that I need make sure to have a consistent API
> > with macvtap.
> >
> > In the case of macvtap, TX means transmit from userspace to kernel, but
> > for TUN, it means transmit from kernel to userspace.
> >
>
> Ah, ok. I'll have to check that out at some point.
>
> >
> > >>
> > >> TUNSETSTEERINGEBPF is a useful bpf hookpoint, but has some drawbacks.
> > >>
> > >> First, it only works on tun/tap devices.
> > >>
> > >> Second, there is no way in the current TUNSETSTEERINGEBPF implementation
> > >> to bail out or load a noop bpf prog and fallback to the no prog tx queue
> > >> selection method.
> >
> >
> > I believe it expect that eBPF should take all the parts (even the
> > fallback part).
> >
>
> This would be easy to change in the existing TUNSETSTEERINGEBPF
> implementation if desired. We'd just need a negative return from the bpf prog
> to result in falling back to tun_automq_select_queue(). If that behavior
> sounds reasonable to you, I can look into that as a separate patch.
>
> >
> > >>
> > >> Third, the TUNSETSTEERINGEBPF interface seems to require possession of existing
> > >> or creation of new queues/fds.
> >
> >
> > That's the way TUN work for past +10 years because ioctl is the only way
> > to do configuration and it requires a fd to carry that. David suggest to
> > implement netlink but nobody did that.
> >
>
> I see.
>
> >
> > >>
> > >> This most naturally fits in the "wire" implementation since possession of fds
> > >> is ensured. However, it also means the various "wire" implementations (e.g.
> > >> qemu) have to all be made aware of TUNSETSTEERINGEBPF and expose an interface
> > >> to load/unload a bpf prog (or provide a mechanism to pass an fd to another
> > >> program).
> >
> >
> > The load/unload of ebpf program is standard bpf() syscall. Ioctl just
> > attach that to TUN. This idea is borrowed from packet socket which the
> > bpf program was attached through setsockopt().
> >
>
> Yeah, it doesn't take much code to load a prog. I wrote one earlier this week
> in fact which spins up an extra fd and detaches right after.
>
> >
> > >>
> > >> Alternatively, you can spin up an extra queue and immediately disable via
> > >> IFF_DETACH_QUEUE, but this seems unsafe; packets could be enqueued to this
> > >> extra file descriptor which is part of our bpf prog loader, not our "wire".
> >
> >
> > You can use you 'wire' queue to do ioctl, but we can invent other API.
> >
>
> It might be cool to provide a way to create an already detached fd
> (not sure if this
> is non-trivial for some reason). Switching over to netlink could be
> the more long
> term goal.
>
> >
> > >>
> > >> Placing this in the XPS code and leveraging iproute2 and rtnetlink to provide
> > >> our bpf prog loader in a similar manner to xdp gives us a nice way to separate
> > >> the tap "wire" and the loading of tx queue selection policy. It also lets us
> > >> use this hookpoint for any device traversing XPS.
> > >>
> > >> This patch only introduces the new hookpoint to the XPS code and will not yet
> > >> be used by tun/tap devices using the intree tun.ko (which implements an
> > >> .ndo_select_queue and does not traverse the XPS code).
> > >>
> > >> In a future patch set, we can optionally refactor tun.ko to traverse this call
> > >> to bpf_prog_run_clear_cb() and bpf prog storage. tun/tap devices could then
> > >> leverage iproute2 as a generic loader. The TUNSETSTEERINGEBPF interface could
> > >> at this point be optionally deprecated/removed.
> >
> >
> > As described above, we need it for macvtap and you propose here can not
> > work for that.
> >
> > I'm not against this proposal, just want to clarify some considerations
> > when developing TUNSETSTEERINGEPF. The main goal is for VM to implement
> > sophisticated steering policy like RSS without touching kernel.
> >
>
> Very cool. Thank you for your comments Jason; they have added clarity
> to some things.
>
> I'm still interested in adding this hookpoint, community willing. I
> believe it provides
> value beyond xps_cpus/xps_rxqs.
>
> I also plan to look into adding a similar hookpoint in the rps code.
> That will unlock
> additional possibilities for this xps hookpoint (e.g. rfs implemented
> via bpf maps, but
> only on a subset of traffic [high priority or especially resource
> costly] rather than all).
>
> I've had (so far casual) chats with a couple NIC vendors about various
> "SmartNICs" supporting custom entropy fields for RSS. I'm playing with the idea
> of an "rpsoffload" prog loaded into the NIC being the way custom entropy is
> configured. Being able to configure RSS to generate a hash based on an fields
> of an inner packet or a packet type specific field like GRE key would be super
> nice for NFV workloads.
>
Turns out the RSS part is already being done via XDP!
https://github.com/Netronome/bpf-samples/tree/master/programmable_rss
> Perhaps even an "rpsdrv" or "rpsoffload" hookpoint could leverage bpf
> helpers for
> RSS hash algorithm (e.g. bfp_rss_hash_toeplitz(), bpf_rss_hash_crc(),
> bpf_rss_hash_xor(), etc.).
>
> The ideas on how things would look for receive are still early, but I
> think there is
> a lot of potential for making things more flexible by leveraging ebpf
> in this area.
>
> > Thanks
> >
> >
> > >>
> > >> Both patches in this set have been tested using a rebuilt tun.ko with no
> > >> .ndo_select_queue.
> > >>
> > >> sed -i '/\.ndo_select_queue.*=/d' drivers/net/tun.c
> > >>
> > >> The tap device was instantiated using tap_mq_pong.c, supporting scripts, and
> > >> wrapping service found here:
> > >>
> > >> https://github.com/stackpath/rxtxcpu/tree/v1.2.6/helpers
> > >>
> > >> The bpf prog source and test scripts can be found here:
> > >>
> > >> https://github.com/werekraken/xps_ebpf
> > >>
> > >> In nstxq, netsniff-ng using PACKET_FANOUT_QM is leveraged to check the
> > >> queue_mapping.
> > >>
> > >> With no prog loaded, the tx queue selection is adhering our xps_cpus
> > >> configuration.
> > >>
> > >> [vagrant@localhost ~]$ grep . /sys/class/net/tap0/queues/tx-*/xps_cpus; ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe;
> > >> /sys/class/net/tap0/queues/tx-0/xps_cpus:1
> > >> /sys/class/net/tap0/queues/tx-1/xps_cpus:2
> > >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.146 ms
> > >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.121 ms
> > >> cpu1: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >>
> > >> With a return 0 bpg prog, our tx queue is 0 (despite xps_cpus).
> > >>
> > >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello0.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> > >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.160 ms
> > >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.124 ms
> > >> cpu1: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> ping-4852 [000] .... 2691.633260: 0: xps (RET 0): Hello, World!
> > >> ping-4869 [001] .... 2695.753588: 0: xps (RET 0): Hello, World!
> > >>
> > >> With a return 1 bpg prog, our tx queue is 1.
> > >>
> > >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello1.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> > >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.193 ms
> > >> cpu0: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.135 ms
> > >> cpu1: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> ping-4894 [000] .... 2710.652080: 0: xps (RET 1): Hello, World!
> > >> ping-4911 [001] .... 2714.774608: 0: xps (RET 1): Hello, World!
> > >>
> > >> With a return 2 bpg prog, our tx queue is 0 (we only have 2 tx queues).
> > >>
> > >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello2.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> > >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=1.20 ms
> > >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.986 ms
> > >> cpu1: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> ping-4936 [000] .... 2729.442668: 0: xps (RET 2): Hello, World!
> > >> ping-4953 [001] .... 2733.614558: 0: xps (RET 2): Hello, World!
> > >>
> > >> With a return -1 bpf prog, our tx queue selection is once again determined by
> > >> xps_cpus. Any negative return should work the same and provides a nice
> > >> mechanism to bail out or have a noop bpf prog at this hookpoint.
> > >>
> > >> [vagrant@localhost ~]$ sudo ip link set dev tap0 xps obj hello_neg1.o sec hello && { ./nstxq; sudo timeout 1 cat /sys/kernel/debug/tracing/trace_pipe; }
> > >> cpu0: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.628 ms
> > >> cpu0: qm0: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> cpu1: ping: 64 bytes from 169.254.254.1: icmp_seq=1 ttl=64 time=0.322 ms
> > >> cpu1: qm1: > tap0 98 Unknown => Unknown IPv4 169.254.254.2/169.254.254.1 Len 84 Type 8 Code 0
> > >> ping-4981 [000] .... 2763.510760: 0: xps (RET -1): Hello, World!
> > >> ping-4998 [001] .... 2767.632583: 0: xps (RET -1): Hello, World!
> > >>
> > >> bpf prog unloading is not yet working and neither does `ip link show` report
> > >> when an "xps" bpf prog is attached. This is my first time touching iproute2 or
> > >> rtnetlink, so it may be something obvious to those more familiar.
> > > Adding Jason... sorry for missing that the first time.
^ permalink raw reply
* [PATCH 0/5] net: ethernet: stmmac: some fixes and optimization
From: Christophe Roullier @ 2019-09-20 5:38 UTC (permalink / raw)
To: robh, davem, joabreu, mark.rutland, mcoquelin.stm32,
alexandre.torgue, peppe.cavallaro
Cc: linux-stm32, linux-kernel, devicetree, linux-arm-kernel, netdev,
christophe.roullier, andrew
Some improvements (manage syscfg as optional clock, update slew rate of
ETH_MDIO pin, Enable gating of the MAC TX clock during TX low-power mode)
Fix warning build message when W=1
Christophe Roullier (5):
net: ethernet: stmmac: Add support for syscfg clock
net: ethernet: stmmac: fix warning when w=1 option is used during
build
ARM: dts: stm32: remove syscfg clock on stm32mp157c ethernet
ARM: dts: stm32: adjust slew rate for Ethernet
ARM: dts: stm32: Enable gating of the MAC TX clock during TX low-power
mode on stm32mp157c
arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 9 +++-
arch/arm/boot/dts/stm32mp157c.dtsi | 7 ++--
.../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 42 ++++++++++++-------
3 files changed, 38 insertions(+), 20 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH 3/5] ARM: dts: stm32: remove syscfg clock on stm32mp157c ethernet
From: Christophe Roullier @ 2019-09-20 5:38 UTC (permalink / raw)
To: robh, davem, joabreu, mark.rutland, mcoquelin.stm32,
alexandre.torgue, peppe.cavallaro
Cc: linux-stm32, linux-kernel, devicetree, linux-arm-kernel, netdev,
christophe.roullier, andrew
In-Reply-To: <20190920053817.13754-1-christophe.roullier@st.com>
Syscfg is now activated automatically when syscfg registers are used
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
arch/arm/boot/dts/stm32mp157c.dtsi | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 0c4e6ebc3529..f51d6222a0e8 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -1285,13 +1285,11 @@
clock-names = "stmmaceth",
"mac-clk-tx",
"mac-clk-rx",
- "ethstp",
- "syscfg-clk";
+ "ethstp";
clocks = <&rcc ETHMAC>,
<&rcc ETHTX>,
<&rcc ETHRX>,
- <&rcc ETHSTP>,
- <&rcc SYSCFG>;
+ <&rcc ETHSTP>;
st,syscon = <&syscfg 0x4>;
snps,mixed-burst;
snps,pbl = <2>;
--
2.17.1
^ permalink raw reply related
* [PATCH 4/5] ARM: dts: stm32: adjust slew rate for Ethernet
From: Christophe Roullier @ 2019-09-20 5:38 UTC (permalink / raw)
To: robh, davem, joabreu, mark.rutland, mcoquelin.stm32,
alexandre.torgue, peppe.cavallaro
Cc: linux-stm32, linux-kernel, devicetree, linux-arm-kernel, netdev,
christophe.roullier, andrew
In-Reply-To: <20190920053817.13754-1-christophe.roullier@st.com>
ETH_MDIO slew-rate should be set to "0" instead of "2"
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
index df6470133574..7667fe758957 100644
--- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
@@ -239,13 +239,18 @@
<STM32_PINMUX('C', 2, AF11)>, /* ETH_RGMII_TXD2 */
<STM32_PINMUX('E', 2, AF11)>, /* ETH_RGMII_TXD3 */
<STM32_PINMUX('B', 11, AF11)>, /* ETH_RGMII_TX_CTL */
- <STM32_PINMUX('A', 2, AF11)>, /* ETH_MDIO */
<STM32_PINMUX('C', 1, AF11)>; /* ETH_MDC */
bias-disable;
drive-push-pull;
- slew-rate = <3>;
+ slew-rate = <2>;
};
pins2 {
+ pinmux = <STM32_PINMUX('A', 2, AF11)>; /* ETH_MDIO */
+ bias-disable;
+ drive-push-pull;
+ slew-rate = <0>;
+ };
+ pins3 {
pinmux = <STM32_PINMUX('C', 4, AF11)>, /* ETH_RGMII_RXD0 */
<STM32_PINMUX('C', 5, AF11)>, /* ETH_RGMII_RXD1 */
<STM32_PINMUX('B', 0, AF11)>, /* ETH_RGMII_RXD2 */
--
2.17.1
^ permalink raw reply related
* [PATCH 1/5] net: ethernet: stmmac: Add support for syscfg clock
From: Christophe Roullier @ 2019-09-20 5:38 UTC (permalink / raw)
To: robh, davem, joabreu, mark.rutland, mcoquelin.stm32,
alexandre.torgue, peppe.cavallaro
Cc: linux-stm32, linux-kernel, devicetree, linux-arm-kernel, netdev,
christophe.roullier, andrew
In-Reply-To: <20190920053817.13754-1-christophe.roullier@st.com>
Add optional support for syscfg clock in dwmac-stm32.c
Now Syscfg clock is activated automatically when syscfg
registers are used
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 36 +++++++++++++------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 4ef041bdf6a1..7e6619868cc1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -152,23 +152,32 @@ static int stm32mp1_clk_prepare(struct stm32_dwmac *dwmac, bool prepare)
int ret = 0;
if (prepare) {
- ret = clk_prepare_enable(dwmac->syscfg_clk);
- if (ret)
- return ret;
-
+ if (dwmac->syscfg_clk) {
+ ret = clk_prepare_enable(dwmac->syscfg_clk);
+ if (ret)
+ return ret;
+ }
if (dwmac->clk_eth_ck) {
ret = clk_prepare_enable(dwmac->clk_eth_ck);
if (ret) {
- clk_disable_unprepare(dwmac->syscfg_clk);
+ if (dwmac->syscfg_clk)
+ goto unprepare_syscfg;
return ret;
}
}
} else {
- clk_disable_unprepare(dwmac->syscfg_clk);
+ if (dwmac->syscfg_clk)
+ clk_disable_unprepare(dwmac->syscfg_clk);
+
if (dwmac->clk_eth_ck)
clk_disable_unprepare(dwmac->clk_eth_ck);
}
return ret;
+
+unprepare_syscfg:
+ clk_disable_unprepare(dwmac->syscfg_clk);
+
+ return ret;
}
static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
@@ -296,7 +305,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
{
struct platform_device *pdev = to_platform_device(dev);
struct device_node *np = dev->of_node;
- int err = 0;
+ int err;
/* Gigabit Ethernet 125MHz clock selection. */
dwmac->eth_clk_sel_reg = of_property_read_bool(np, "st,eth-clk-sel");
@@ -320,13 +329,17 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
return PTR_ERR(dwmac->clk_ethstp);
}
- /* Clock for sysconfig */
+ /* Optional Clock for sysconfig */
dwmac->syscfg_clk = devm_clk_get(dev, "syscfg-clk");
if (IS_ERR(dwmac->syscfg_clk)) {
- dev_err(dev, "No syscfg clock provided...\n");
- return PTR_ERR(dwmac->syscfg_clk);
+ err = PTR_ERR(dwmac->syscfg_clk);
+ if (err != -ENOENT)
+ return err;
+ dwmac->syscfg_clk = NULL;
}
+ err = 0;
+
/* Get IRQ information early to have an ability to ask for deferred
* probe if needed before we went too far with resource allocation.
*/
@@ -436,7 +449,8 @@ static int stm32mp1_suspend(struct stm32_dwmac *dwmac)
return ret;
clk_disable_unprepare(dwmac->clk_tx);
- clk_disable_unprepare(dwmac->syscfg_clk);
+ if (dwmac->syscfg_clk)
+ clk_disable_unprepare(dwmac->syscfg_clk);
if (dwmac->clk_eth_ck)
clk_disable_unprepare(dwmac->clk_eth_ck);
--
2.17.1
^ permalink raw reply related
* [PATCH 2/5] net: ethernet: stmmac: fix warning when w=1 option is used during build
From: Christophe Roullier @ 2019-09-20 5:38 UTC (permalink / raw)
To: robh, davem, joabreu, mark.rutland, mcoquelin.stm32,
alexandre.torgue, peppe.cavallaro
Cc: linux-stm32, linux-kernel, devicetree, linux-arm-kernel, netdev,
christophe.roullier, andrew
In-Reply-To: <20190920053817.13754-1-christophe.roullier@st.com>
This patch fix the following warning:
warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
int val, ret;
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 7e6619868cc1..167a5e99960a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -184,7 +184,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
{
struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
u32 reg = dwmac->mode_reg;
- int val, ret;
+ int val;
switch (plat_dat->interface) {
case PHY_INTERFACE_MODE_MII:
@@ -220,8 +220,8 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
}
/* Need to update PMCCLRR (clear register) */
- ret = regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
- dwmac->ops->syscfg_eth_mask);
+ regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
+ dwmac->ops->syscfg_eth_mask);
/* Update PMCSETR (set register) */
return regmap_update_bits(dwmac->regmap, reg,
--
2.17.1
^ permalink raw reply related
* [PATCH 5/5] ARM: dts: stm32: Enable gating of the MAC TX clock during TX low-power mode on stm32mp157c
From: Christophe Roullier @ 2019-09-20 5:38 UTC (permalink / raw)
To: robh, davem, joabreu, mark.rutland, mcoquelin.stm32,
alexandre.torgue, peppe.cavallaro
Cc: linux-stm32, linux-kernel, devicetree, linux-arm-kernel, netdev,
christophe.roullier, andrew
In-Reply-To: <20190920053817.13754-1-christophe.roullier@st.com>
When there is no activity on ethernet phy link, the ETH_GTX_CLK is cut
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index f51d6222a0e8..d78dfc44a1fb 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -1293,6 +1293,7 @@
st,syscon = <&syscfg 0x4>;
snps,mixed-burst;
snps,pbl = <2>;
+ snps,en-tx-lpi-clockgating;
snps,axi-config = <&stmmac_axi_config_0>;
snps,tso;
status = "disabled";
--
2.17.1
^ permalink raw reply related
* Re: mt76x2e hardware restart
From: Oleksandr Natalenko @ 2019-09-20 6:07 UTC (permalink / raw)
To: linux-mediatek
Cc: Felix Fietkau, Lorenzo Bianconi, Lorenzo Bianconi,
Stanislaw Gruszka, Ryder Lee, Roy Luo, Kalle Valo,
David S. Miller, Matthias Brugger, linux-wireless, netdev,
linux-arm-kernel, linux-kernel
In-Reply-To: <c6d621759c190f7810d898765115f3b4@natalenko.name>
On 19.09.2019 23:22, Oleksandr Natalenko wrote:
> It checks for TX hang here:
>
> === mt76x02_mmio.c
> 557 void mt76x02_wdt_work(struct work_struct *work)
> 558 {
> ...
> 562 mt76x02_check_tx_hang(dev);
> ===
I've commented out the watchdog here ^^, and the card is not resetted
any more, but similarly it stops working shortly after the first client
connects. So, indeed, it must be some hang in the HW, and wdt seems to
do a correct job.
Is it even debuggable/fixable from the driver?
--
Oleksandr Natalenko (post-factum)
^ permalink raw reply
* Re: [PATCH net v2 1/3] net: sched: sch_htb: don't call qdisc_put() while holding tree lock
From: Vlad Buslov @ 2019-09-20 6:27 UTC (permalink / raw)
To: Cong Wang
Cc: Vlad Buslov, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko, David Miller
In-Reply-To: <CAM_iQpWREfLQX6VSqLw_xTm8WkNBZ8_adGWE5PpTnVQVDBWPvw@mail.gmail.com>
On Fri 20 Sep 2019 at 04:05, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Sep 19, 2019 at 1:14 PM Vlad Buslov <vladbu@mellanox.com> wrote:
>> Notes:
>> Changes V1 -> V2:
>>
>> - Extend sch API with new qdisc_put_empty() function that has same
>> implementation as regular qdisc_put() but skips parts that reset qdisc
>> and free all packet buffers from gso_skb and skb_bad_txq queues.
>
> I don't understand why you need a new API here, as long as qdisc_reset()
> gets called before releasing sch tree lock, the ->reset() inside qdisc_put(),
> after releasing sch tree lock, should be a nop, right?
Yes, but I wanted to make it explicit, so anyone else looking at the
code of those Qdiscs would know that manual reset with appropriate
locking is required. And it didn't require much new code because
qdisc_put() and qidsc_put_empty() just reuse same __qdisc_put(). I'll
revert it back, if you suggest that original approach is better.
^ permalink raw reply
* Re: [PATCH net v2 1/3] net: sched: sch_htb: don't call qdisc_put() while holding tree lock
From: Vlad Buslov @ 2019-09-20 6:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Vlad Buslov, netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net
In-Reply-To: <66e68933-a553-e078-b92b-6f629c740328@gmail.com>
On Fri 20 Sep 2019 at 01:49, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 9/19/19 1:14 PM, Vlad Buslov wrote:
>> Recent changes that removed rtnl dependency from rules update path of tc
>> also made tcf_block_put() function sleeping. This function is called from
>> ops->destroy() of several Qdisc implementations, which in turn is called by
>> qdisc_put(). Some Qdiscs call qdisc_put() while holding sch tree spinlock,
>> which results sleeping-while-atomic BUG.
>>
>
>
> Note that calling qdisc_put() while holding sch tree lock can also
> trigger deadlocks.
>
> For example sch_pie.c has a del_timer_sync() in pie_destroy(),
> while the pie_timer() timer handler acquires the root_lock.
>
> (there are other cases like that, SFQ for example)
These and other examples of sleeping calls in cls APIs used by
tcf_block_put() that I described in one of my previous emails make me
think that putting might_sleep() at the beginning of qdisc_put() would
be a good idea, instead of waiting for syzbot to find correct
combination to trigger a crash.
^ permalink raw reply
* Re: [PATCH 2/2][ethtool] ethtool: implement support for Energy Detect Power Down
From: Ardelean, Alexandru @ 2019-09-20 6:39 UTC (permalink / raw)
To: andrew@lunn.ch
Cc: netdev@vger.kernel.org, linville@tuxdriver.com,
f.fainelli@gmail.com
In-Reply-To: <20190919140025.GC22556@lunn.ch>
On Thu, 2019-09-19 at 16:00 +0200, Andrew Lunn wrote:
> [External]
>
> > -static int parse_named_u8(struct cmd_context *ctx, const char *name,
> > u8 *val)
> > +static int parse_named_uint(struct cmd_context *ctx, const char *name,
> > + void *val, enum tunable_type_id type_id)
> > {
> > if (ctx->argc < 2)
> > return 0;
> > @@ -5026,7 +5051,16 @@ static int parse_named_u8(struct cmd_context
> > *ctx, const char *name, u8 *val)
> > if (strcmp(*ctx->argp, name))
> > return 0;
> >
> > - *val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
> > + switch (type_id) {
> > + case ETHTOOL_TUNABLE_U8:
> > + *(u8 *)val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
> > + break;
> > + case ETHTOOL_TUNABLE_U16:
> > + *(u16 *)val = get_uint_range(*(ctx->argp + 1), 0, 0xffff);
>
> I personally don't like these casts. Could you refactor this code in
> some other way to avoid them. Make the parse_named_u8()
> parse_named_u16() a bit fatter, and the shared code a bit slimmer?
>
Sure thing.
V2 coming shortly.
> Thanks
> Andrew
^ permalink raw reply
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