* Re: [RFC v4 0/3] vhost: introduce mdev based hardware backend
From: Jason Wang @ 2019-09-20 1:30 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: <20190919154552.GA27657@___>
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,
}
>
> 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. 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.
Thanks
>
> Thanks,
> Tiwei
>
>> Thanks
>>
>>
>>>> Yes, it is.
>>>>
>>>> Thanks
>>>>
>>>>
^ permalink raw reply
* Re: [PATCH] net/ncsi: Disable global multicast filter
From: Jakub Kicinski @ 2019-09-20 1:32 UTC (permalink / raw)
To: Vijay Khemka
Cc: Samuel Mendoza-Jonas, David S. Miller, netdev, linux-kernel,
openbmc @ lists . ozlabs . org, joel, linux-aspeed, sdasari
In-Reply-To: <20190912190451.2362220-1-vijaykhemka@fb.com>
On Thu, 12 Sep 2019 12:04:50 -0700, Vijay Khemka wrote:
> Disabling multicast filtering from NCSI if it is supported. As it
> should not filter any multicast packets. In current code, multicast
> filter is enabled and with an exception of optional field supported
> by device are disabled filtering.
>
> Mainly I see if goal is to disable filtering for IPV6 packets then let
> it disabled for every other types as well. As we are seeing issues with
> LLDP not working with this enabled filtering. And there are other issues
> with IPV6.
>
> By Disabling this multicast completely, it is working for both IPV6 as
> well as LLDP.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> @@ -1033,23 +1030,23 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> } else if (nd->state == ncsi_dev_state_config_ebf) {
> nca.type = NCSI_PKT_CMD_EBF;
> nca.dwords[0] = nc->caps[NCSI_CAP_BC].cap;
> - if (ncsi_channel_is_tx(ndp, nc))
> + /* if multicast global filtering is supported then
> + * disable it so that all multicast packet will be
> + * forwarded to management controller
> + */
> + if (nc->caps[NCSI_CAP_GENERIC].cap &
> + NCSI_CAP_GENERIC_MC)
Applied, looks like an unnecessary space sneaked in here, I removed it.
^ permalink raw reply
* Re: [PATCH] selftests: Add test cases for `ip nexthop flush proto XX`
From: Jakub Kicinski @ 2019-09-20 1:36 UTC (permalink / raw)
To: Donald Sharp; +Cc: netdev, dsahern
In-Reply-To: <20190916122650.24124-1-sharpd@cumulusnetworks.com>
On Mon, 16 Sep 2019 08:26:50 -0400, Donald Sharp wrote:
> Add some test cases to allow the fib_nexthops.sh test code
> to test the flushing of nexthops based upon the proto passed
> in upon creation of the nexthop group.
>
> Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Applied, thank you!
^ permalink raw reply
* [PATCH] ath10k: fix memory leak
From: Navid Emamdoost @ 2019-09-20 1:36 UTC (permalink / raw)
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Kalle Valo,
David S. Miller, ath10k, linux-wireless, netdev, linux-kernel
In ath10k_usb_hif_tx_sg the allocated urb should be released if
usb_submit_urb fails.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/wireless/ath/ath10k/usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/ath10k/usb.c b/drivers/net/wireless/ath/ath10k/usb.c
index e1420f67f776..730ed22e08a0 100644
--- a/drivers/net/wireless/ath/ath10k/usb.c
+++ b/drivers/net/wireless/ath/ath10k/usb.c
@@ -435,6 +435,7 @@ static int ath10k_usb_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
ath10k_dbg(ar, ATH10K_DBG_USB_BULK,
"usb bulk transmit failed: %d\n", ret);
usb_unanchor_urb(urb);
+ usb_free_urb(urb);
ret = -EINVAL;
goto err_free_urb_to_pipe;
}
--
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: Jason Wang @ 2019-09-20 1:42 UTC (permalink / raw)
To: Matt Cover, 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_hrkUPxBAOttpo+xqcUHr89i2N5BOV4_rYFh=dRPkYVPGA@mail.gmail.com>
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.
>>
>> 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).
>>
>> 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.
>>
>> 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().
>>
>> 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.
>>
>> 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.
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
* Re: [PATCH iproute2 master] bpf: Fix race condition with map pinning
From: Joe Stringer @ 2019-09-20 1:56 UTC (permalink / raw)
To: Joe Stringer; +Cc: Stephen Hemminger, netdev, Daniel Borkmann
In-Reply-To: <20190919220733.31206-1-joe@wand.net.nz>
On Thu, Sep 19, 2019 at 3:07 PM Joe Stringer <joe@wand.net.nz> wrote:
>
> If two processes attempt to invoke bpf_map_attach() at the same time,
> then they will both create maps, then the first will successfully pin
> the map to the filesystem and the second will not pin the map, but will
> continue operating with a reference to its own copy of the map. As a
> result, the sharing of the same map will be broken from the two programs
> that were concurrently loaded via loaders using this library.
>
> Fix this by adding a retry in the case where the pinning fails because
> the map already exists on the filesystem. In that case, re-attempt
> opening a fd to the map on the filesystem as it shows that another
> program already created and pinned a map at that location.
>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> ---
> lib/bpf.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lib/bpf.c b/lib/bpf.c
> index f64b58c3bb19..23eb8952cc28 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -1625,7 +1625,9 @@ static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx,
> int *have_map_in_map)
> {
> int fd, ifindex, ret, map_inner_fd = 0;
> + bool retried = false;
>
> +probe:
> fd = bpf_probe_pinned(name, ctx, map->pinning);
> if (fd > 0) {
> ret = bpf_map_selfcheck_pinned(fd, map, ext,
> @@ -1674,7 +1676,11 @@ static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx,
> }
>
> ret = bpf_place_pinned(fd, name, ctx, map->pinning);
> - if (ret < 0 && errno != EEXIST) {
> + if (ret < 0) {
> + if (!retried && errno == EEXIST) {
> + retried = true;
> + goto probe;
> + }
Ah, forgot to close 'fd' before the jump in this retry case. Will fix
that up in v2.
^ permalink raw reply
* [PATCHv2 iproute2 master] bpf: Fix race condition with map pinning
From: Joe Stringer @ 2019-09-20 2:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, daniel
If two processes attempt to invoke bpf_map_attach() at the same time,
then they will both create maps, then the first will successfully pin
the map to the filesystem and the second will not pin the map, but will
continue operating with a reference to its own copy of the map. As a
result, the sharing of the same map will be broken from the two programs
that were concurrently loaded via loaders using this library.
Fix this by adding a retry in the case where the pinning fails because
the map already exists on the filesystem. In that case, re-attempt
opening a fd to the map on the filesystem as it shows that another
program already created and pinned a map at that location.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
v2: Fix close of created map in the EEXIST case.
v1: Original patch
---
lib/bpf.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index 01152b26e54a..86ab0698660f 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -1707,7 +1707,9 @@ static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx,
int *have_map_in_map)
{
int fd, ifindex, ret, map_inner_fd = 0;
+ bool retried = false;
+probe:
fd = bpf_probe_pinned(name, ctx, map->pinning);
if (fd > 0) {
ret = bpf_map_selfcheck_pinned(fd, map, ext,
@@ -1756,10 +1758,14 @@ static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx,
}
ret = bpf_place_pinned(fd, name, ctx, map->pinning);
- if (ret < 0 && errno != EEXIST) {
+ if (ret < 0) {
+ close(fd);
+ if (!retried && errno == EEXIST) {
+ retried = true;
+ goto probe;
+ }
fprintf(stderr, "Could not pin %s map: %s\n", name,
strerror(errno));
- close(fd);
return ret;
}
--
2.20.1
^ permalink raw reply related
* 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
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