* 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] dt-bindings: net: Correct the documentation of KSZ9021 skew values
From: Jakub Kicinski @ 2019-09-20 1:30 UTC (permalink / raw)
To: James Byrne
Cc: David Miller, robh+dt@kernel.org, mark.rutland@arm.com,
netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <0102016d3b297538-fcca5199-6ad1-4625-b11c-3ad3919a0c48-000000@eu-west-1.amazonses.com>
On Mon, 16 Sep 2019 17:40:35 +0000, James Byrne wrote:
> On 16/09/2019 15:14, David Miller wrote:
> > From: James Byrne <james.byrne@origamienergy.com>
> > Date: Fri, 13 Sep 2019 16:46:35 +0000
> >
> >> The documentation of skew values for the KSZ9021 PHY was misleading
> >> because the driver implementation followed the erroneous information
> >> given in the original KSZ9021 datasheet before it was corrected in
> >> revision 1.2 (Feb 2014). It is probably too late to correct the driver
> >> now because of the many existing device trees, so instead this just
> >> corrects the documentation to explain that what you actually get is not
> >> what you might think when looking at the device tree.
> >>
> >> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
> >
> > What tree should this go into?
>
> I believe this should go into the 'net' tree, but please let me know if
> I have submitted this patch incorrectly in some way.
Okay, applied, thanks.
^ permalink raw reply
* Re: [PATCH net v2 1/3] net: sched: sch_htb: don't call qdisc_put() while holding tree lock
From: Cong Wang @ 2019-09-20 1:05 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190919201438.2383-2-vladbu@mellanox.com>
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?
^ permalink raw reply
* Re: [RFC v4 0/3] vhost: introduce mdev based hardware backend
From: Jason Wang @ 2019-09-20 0:59 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 ...
>
> return vfio_add_group_dev(dev, &vfio_vhost_mdev_dev_ops, mdev);
> }
>
> 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.
>
> Thanks,
> Tiwei
Yes, something like this.
Thanks
>> Thanks
>>
>>
>>>> Yes, it is.
>>>>
>>>> Thanks
>>>>
>>>>
^ permalink raw reply
* Re: ionic build issues?
From: Jakub Kicinski @ 2019-09-20 0:50 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Shannon Nelson, netdev
In-Reply-To: <a99cf2d8-2479-2003-ac0b-10d44363c872@infradead.org>
On Thu, 19 Sep 2019 17:29:28 -0700, Randy Dunlap wrote:
> On 9/19/19 5:27 PM, Jakub Kicinski wrote:
> > Hi Shannon!
> >
> > I've enabled ionic to do some build testing before pushing patches
> > today, and with my tree which I build with W=1 C=1 I run into this:
> >
> > ../drivers/net/ethernet/pensando/ionic/ionic_main.c: In function ‘ionic_adminq_cb’:
> > ../drivers/net/ethernet/pensando/ionic/ionic_main.c:229:2: error: implicit declaration of function ‘dynamic_hex_dump’ [-Werror=implicit-function-declaration]
> > 229 | dynamic_hex_dump("comp ", DUMP_PREFIX_OFFSET, 16, 1,
> > | ^~~~~~~~~~~~~~~~
> > ../drivers/net/ethernet/pensando/ionic/ionic_lif.c: In function ‘ionic_notifyq_service’:
> > ../drivers/net/ethernet/pensando/ionic/ionic_lif.c:673:2: error: implicit declaration of function ‘dynamic_hex_dump’ [-Werror=implicit-function-declaration]
> > 673 | dynamic_hex_dump("event ", DUMP_PREFIX_OFFSET, 16, 1,
> > | ^~~~~~~~~~~~~~~~
> > cc1: some warnings being treated as errors
> >
> > Config attached, could you please take a look?
> >
>
> Patch is already posted.
> See https://lore.kernel.org/netdev/20190918195607.2080036-1-arnd@arndb.de/
Ah, thanks for pointing that out.
^ permalink raw reply
* Re: [PATCH net] ibmvnic: Warn unknown speed message only when carrier is present
From: Jakub Kicinski @ 2019-09-20 0:37 UTC (permalink / raw)
To: Murilo Fossa Vicentini; +Cc: netdev, tlfalcon, muvic, abdhalee
In-Reply-To: <20190916145037.77376-1-muvic@linux.ibm.com>
On Mon, 16 Sep 2019 11:50:37 -0300, Murilo Fossa Vicentini wrote:
> With commit 0655f9943df2 ("net/ibmvnic: Update carrier state after link
> state change") we are now able to detect when the carrier is properly
> present in the device, so only report an unexpected unknown speed when it
> is properly detected. Unknown speed is expected to be seen by the device
> in case the backing device has no link detected.
>
> Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Tested-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Signed-off-by: Murilo Fossa Vicentini <muvic@linux.ibm.com>
Applied, thanks!
^ permalink raw reply
* Re: ionic build issues?
From: Randy Dunlap @ 2019-09-20 0:29 UTC (permalink / raw)
To: Jakub Kicinski, Shannon Nelson; +Cc: netdev
In-Reply-To: <20190919172739.0c6b4bc4@cakuba.netronome.com>
On 9/19/19 5:27 PM, Jakub Kicinski wrote:
> Hi Shannon!
>
> I've enabled ionic to do some build testing before pushing patches
> today, and with my tree which I build with W=1 C=1 I run into this:
>
> ../drivers/net/ethernet/pensando/ionic/ionic_main.c: In function ‘ionic_adminq_cb’:
> ../drivers/net/ethernet/pensando/ionic/ionic_main.c:229:2: error: implicit declaration of function ‘dynamic_hex_dump’ [-Werror=implicit-function-declaration]
> 229 | dynamic_hex_dump("comp ", DUMP_PREFIX_OFFSET, 16, 1,
> | ^~~~~~~~~~~~~~~~
> ../drivers/net/ethernet/pensando/ionic/ionic_lif.c: In function ‘ionic_notifyq_service’:
> ../drivers/net/ethernet/pensando/ionic/ionic_lif.c:673:2: error: implicit declaration of function ‘dynamic_hex_dump’ [-Werror=implicit-function-declaration]
> 673 | dynamic_hex_dump("event ", DUMP_PREFIX_OFFSET, 16, 1,
> | ^~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
>
> Config attached, could you please take a look?
>
Patch is already posted.
See https://lore.kernel.org/netdev/20190918195607.2080036-1-arnd@arndb.de/
--
~Randy
^ permalink raw reply
* ionic build issues?
From: Jakub Kicinski @ 2019-09-20 0:27 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 937 bytes --]
Hi Shannon!
I've enabled ionic to do some build testing before pushing patches
today, and with my tree which I build with W=1 C=1 I run into this:
../drivers/net/ethernet/pensando/ionic/ionic_main.c: In function ‘ionic_adminq_cb’:
../drivers/net/ethernet/pensando/ionic/ionic_main.c:229:2: error: implicit declaration of function ‘dynamic_hex_dump’ [-Werror=implicit-function-declaration]
229 | dynamic_hex_dump("comp ", DUMP_PREFIX_OFFSET, 16, 1,
| ^~~~~~~~~~~~~~~~
../drivers/net/ethernet/pensando/ionic/ionic_lif.c: In function ‘ionic_notifyq_service’:
../drivers/net/ethernet/pensando/ionic/ionic_lif.c:673:2: error: implicit declaration of function ‘dynamic_hex_dump’ [-Werror=implicit-function-declaration]
673 | dynamic_hex_dump("event ", DUMP_PREFIX_OFFSET, 16, 1,
| ^~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
Config attached, could you please take a look?
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27441 bytes --]
^ permalink raw reply
* Re: [PATCH] ionic: remove useless return code
From: Jakub Kicinski @ 2019-09-20 0:21 UTC (permalink / raw)
To: Shannon Nelson
Cc: Arnd Bergmann, Pensando Drivers, David S. Miller, netdev,
linux-kernel, clang-built-linux
In-Reply-To: <6cdb1e21-44d9-bba9-1931-78f7109bff2b@pensando.io>
On Wed, 18 Sep 2019 13:46:34 -0700, Shannon Nelson wrote:
> On 9/18/19 12:57 PM, Arnd Bergmann wrote:
> > The debugfs function was apparently changed from returning an error code
> > to a void return, but the return code left in place, causing a warning
> > from clang:
> >
> > drivers/net/ethernet/pensando/ionic/ionic_debugfs.c:60:37: error: expression result unused [-Werror,-Wunused-value]
> > ionic, &identity_fops) ? 0 : -EOPNOTSUPP;
> > ^~~~~~~~~~~
> >
> > Fixes: fbfb8031533c ("ionic: Add hardware init and device commands")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > drivers/net/ethernet/pensando/ionic/ionic_debugfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> > index 7afc4a365b75..bc03cecf80cc 100644
> > --- a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> > +++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> > @@ -57,7 +57,7 @@ DEFINE_SHOW_ATTRIBUTE(identity);
> > void ionic_debugfs_add_ident(struct ionic *ionic)
> > {
> > debugfs_create_file("identity", 0400, ionic->dentry,
> > - ionic, &identity_fops) ? 0 : -EOPNOTSUPP;
> > + ionic, &identity_fops);
> > }
> >
> > void ionic_debugfs_add_sizes(struct ionic *ionic)
>
> This has just recently been addressed by Nathan Chancellor
> <natechancellor@gmail.com>
Yup, should be in the net tree now.
> Either way,
>
> Acked-by: Shannon Nelson <snelson@pensando.io>
Thanks for quick reviews!
^ permalink raw reply
* Re: [PATCH] ionic: Remove unnecessary ternary operator in ionic_debugfs_add_ident
From: Jakub Kicinski @ 2019-09-20 0:21 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Shannon Nelson, Pensando Drivers, David S. Miller, netdev,
linux-kernel, clang-built-linux, Greg Kroah-Hartman
In-Reply-To: <20190917232616.125261-1-natechancellor@gmail.com>
On Tue, 17 Sep 2019 16:26:16 -0700, Nathan Chancellor wrote:
> clang warns:
>
> ../drivers/net/ethernet/pensando/ionic/ionic_debugfs.c:60:37: warning:
> expression result unused [-Wunused-value]
> ionic, &identity_fops) ? 0 : -EOPNOTSUPP;
> ^~~~~~~~~~~
> 1 warning generated.
>
> The return value of debugfs_create_file does not need to be checked [1]
> and the function returns void so get rid of the ternary operator, it is
> unnecessary.
>
> [1]: https://lore.kernel.org/linux-mm/20150815160730.GB25186@kroah.com/
>
> Fixes: fbfb8031533c ("ionic: Add hardware init and device commands")
> Link: https://github.com/ClangBuiltLinux/linux/issues/658
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Applied, thank you!
^ permalink raw reply
* 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 0:05 UTC (permalink / raw)
To: 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, Jason Wang
In-Reply-To: <20190919224458.91422-1-matthew.cover@stackpath.com>
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.
>
> 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.
>
> Third, the TUNSETSTEERINGEBPF interface seems to require possession of existing
> or creation of new queues/fds.
>
> 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).
>
> 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".
>
> 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.
>
> 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: Daniel Borkmann @ 2019-09-19 23:25 UTC (permalink / raw)
To: Joe Stringer; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20190919220733.31206-1-joe@wand.net.nz>
On Thu, Sep 19, 2019 at 03:07:33PM -0700, Joe Stringer 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>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net v2 1/3] net: sched: sch_htb: don't call qdisc_put() while holding tree lock
From: Eric Dumazet @ 2019-09-19 22:49 UTC (permalink / raw)
To: Vlad Buslov, netdev; +Cc: jhs, xiyou.wangcong, jiri, davem
In-Reply-To: <20190919201438.2383-2-vladbu@mellanox.com>
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)
^ permalink raw reply
* [RFC iproute2-next 2/2] Introduce an eBPF hookpoint for tx queue selection in the XPS (Transmit Packet Steering) code.
From: Matthew Cover @ 2019-09-19 22:46 UTC (permalink / raw)
To: davem, ast, daniel, kafai, songliubraving, yhs, nikolay, sd,
sbrivio, vincent, kda, matthew.cover, jiri, edumazet, pabeni,
idosch, petrm, f.fainelli, stephen, dsahern, christian,
jakub.kicinski, roopa, johannes.berg, mkubecek, netdev,
linux-kernel, bpf
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.
---
include/bpf_api.h | 5 +++
include/uapi/linux/if_link.h | 12 ++++++
ip/Makefile | 2 +-
ip/ip_common.h | 4 ++
ip/iplink.c | 7 ++++
ip/iplink_xps.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
lib/bpf.c | 6 +++
7 files changed, 123 insertions(+), 1 deletion(-)
create mode 100644 ip/iplink_xps.c
diff --git a/include/bpf_api.h b/include/bpf_api.h
index 89d3488..d1a2d90 100644
--- a/include/bpf_api.h
+++ b/include/bpf_api.h
@@ -78,6 +78,11 @@
__section(ELF_SECTION_PROG)
#endif
+#ifndef __section_xps_entry
+# define __section_xps_entry \
+ __section(ELF_SECTION_PROG)
+#endif
+
#ifndef __section_cls_entry
# define __section_cls_entry \
__section(ELF_SECTION_CLASSIFIER)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d36919f..9efd686 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -167,6 +167,7 @@ enum {
IFLA_NEW_IFINDEX,
IFLA_MIN_MTU,
IFLA_MAX_MTU,
+ IFLA_XPS,
__IFLA_MAX
};
@@ -977,6 +978,17 @@ enum {
#define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
+/* XPS section */
+
+enum {
+ IFLA_XPS_UNSPEC,
+ IFLA_XPS_FD,
+ IFLA_XPS_ATTACHED,
+ __IFLA_XPS_MAX,
+};
+
+#define IFLA_XPS_MAX (__IFLA_XPS_MAX - 1)
+
enum {
IFLA_EVENT_NONE,
IFLA_EVENT_REBOOT, /* internal reset / reboot */
diff --git a/ip/Makefile b/ip/Makefile
index 5ab78d7..9ad1c53 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -5,7 +5,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
ipxfrm.o xfrm_state.o xfrm_policy.o xfrm_monitor.o iplink_dummy.o \
iplink_ifb.o iplink_nlmon.o iplink_team.o iplink_vcan.o iplink_vxcan.o \
iplink_vlan.o link_veth.o link_gre.o iplink_can.o iplink_xdp.o \
- iplink_macvlan.o ipl2tp.o link_vti.o link_vti6.o link_xfrm.o \
+ iplink_macvlan.o ipl2tp.o link_vti.o link_vti6.o link_xfrm.o iplink_xps.o \
iplink_vxlan.o tcp_metrics.o iplink_ipoib.o ipnetconf.o link_ip6tnl.o \
link_iptnl.o link_gre6.o iplink_bond.o iplink_bond_slave.o iplink_hsr.o \
iplink_bridge.o iplink_bridge_slave.o ipfou.o iplink_ipvlan.o \
diff --git a/ip/ip_common.h b/ip/ip_common.h
index cd916ec..805d7d2 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -145,6 +145,10 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, const char *ifnam
bool generic, bool drv, bool offload);
void xdp_dump(FILE *fp, struct rtattr *tb, bool link, bool details);
+/* iplink_xps.c */
+int xps_parse(int *argc, char ***argv, struct iplink_req *req);
+void xps_dump(FILE *fp, struct rtattr *tb);
+
/* iplink_vrf.c */
__u32 ipvrf_get_table(const char *name);
int name_is_vrf(const char *name);
diff --git a/ip/iplink.c b/ip/iplink.c
index 212a088..4d6d557 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -101,6 +101,9 @@ void iplink_usage(void)
" [ { xdp | xdpgeneric | xdpdrv | xdpoffload } { off |\n"
" object FILE [ section NAME ] [ verbose ] |\n"
" pinned FILE } ]\n"
+ " [ xps { off |\n"
+ " object FILE [ section NAME ] [ verbose ] |\n"
+ " pinned FILE } ]\n"
" [ master DEVICE ][ vrf NAME ]\n"
" [ nomaster ]\n"
" [ addrgenmode { eui64 | none | stable_secret | random } ]\n"
@@ -668,6 +671,10 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
if (offload && name == dev)
dev = NULL;
+ } else if (strcmp(*argv, "xps") == 0) {
+ NEXT_ARG();
+ if (xps_parse(&argc, &argv, req))
+ exit(-1);
} else if (strcmp(*argv, "netns") == 0) {
NEXT_ARG();
if (netns != -1)
diff --git a/ip/iplink_xps.c b/ip/iplink_xps.c
new file mode 100644
index 0000000..7e94164
--- /dev/null
+++ b/ip/iplink_xps.c
@@ -0,0 +1,88 @@
+/*
+ * iplink_xps.c XPS program loader
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Matthew Cover <matthew.cover@stackpath.com>
+ *
+ * Based on iplink_xdp.c by Daniel Borkmann <daniel@iogearbox.net>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <linux/bpf.h>
+
+#include "bpf_util.h"
+#include "ip_common.h"
+
+extern int force;
+
+struct xps_req {
+ struct iplink_req *req;
+ __u32 flags;
+};
+
+static void xps_ebpf_cb(void *raw, int fd, const char *annotation)
+{
+ struct xps_req *xps = raw;
+ struct iplink_req *req = xps->req;
+ struct rtattr *xps_attr;
+
+ xps_attr = addattr_nest(&req->n, sizeof(*req), IFLA_XPS);
+ addattr32(&req->n, sizeof(*req), IFLA_XPS_FD, fd);
+ addattr_nest_end(&req->n, xps_attr);
+}
+
+static const struct bpf_cfg_ops bpf_cb_ops = {
+ .ebpf_cb = xps_ebpf_cb,
+};
+
+static int xps_delete(struct iplink_req *req)
+{
+ xps_ebpf_cb(req, -1, NULL);
+ return 0;
+}
+
+int xps_parse(int *argc, char ***argv, struct iplink_req *req)
+{
+
+ struct bpf_cfg_in cfg = {
+ .type = BPF_PROG_TYPE_SOCKET_FILTER,
+ .argc = *argc,
+ .argv = *argv,
+ };
+
+ struct xps_req xps = {
+ .req = req,
+ };
+
+ if (*argc == 1) {
+ if (strcmp(**argv, "none") == 0 ||
+ strcmp(**argv, "off") == 0)
+ return xps_delete(req);
+ }
+
+ if (bpf_parse_and_load_common(&cfg, &bpf_cb_ops, &xps))
+ return -1;
+
+ *argc = cfg.argc;
+ *argv = cfg.argv;
+ return 0;
+}
+
+void xps_dump(FILE *fp, struct rtattr *xps)
+{
+ struct rtattr *tb[IFLA_XPS_MAX + 1];
+
+ parse_rtattr_nested(tb, IFLA_XPS_MAX, xps);
+ if (!tb[IFLA_XPS_ATTACHED] ||
+ !rta_getattr_u8(tb[IFLA_XPS_ATTACHED]))
+ return;
+
+ fprintf(fp, "xps ");
+ /* More to come here in future for 'ip -d link' (digest, etc) ... */
+}
diff --git a/lib/bpf.c b/lib/bpf.c
index 7d2a322..e883afb 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -60,6 +60,7 @@ static const enum bpf_prog_type __bpf_types[] = {
BPF_PROG_TYPE_LWT_IN,
BPF_PROG_TYPE_LWT_OUT,
BPF_PROG_TYPE_LWT_XMIT,
+ BPF_PROG_TYPE_SOCKET_FILTER,
};
static const struct bpf_prog_meta __bpf_prog_meta[] = {
@@ -100,6 +101,11 @@ static const struct bpf_prog_meta __bpf_prog_meta[] = {
.subdir = "ip",
.section = ELF_SECTION_PROG,
},
+ [BPF_PROG_TYPE_SOCKET_FILTER] = {
+ .type = "xps",
+ .subdir = "xps",
+ .section = ELF_SECTION_PROG,
+ },
};
static const char *bpf_prog_to_subdir(enum bpf_prog_type type)
--
1.8.3.1
^ permalink raw reply related
* [RFC net-next 1/2] Introduce an eBPF hookpoint for tx queue selection in the XPS (Transmit Packet Steering) code.
From: Matthew Cover @ 2019-09-19 22:45 UTC (permalink / raw)
To: davem, ast, daniel, kafai, songliubraving, yhs, nikolay, sd,
sbrivio, vincent, kda, matthew.cover, jiri, edumazet, pabeni,
idosch, petrm, f.fainelli, stephen, dsahern, christian,
jakub.kicinski, roopa, johannes.berg, mkubecek, netdev,
linux-kernel, bpf
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.
---
include/linux/netdevice.h | 3 +++
include/uapi/linux/if_link.h | 12 +++++++++
net/core/dev.c | 61 ++++++++++++++++++++++++++++++++++++-------
net/core/rtnetlink.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 129 insertions(+), 9 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9eda1c3..88e37d5 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1966,6 +1966,7 @@ struct net_device {
#ifdef CONFIG_XPS
struct xps_dev_maps __rcu *xps_cpus_map;
struct xps_dev_maps __rcu *xps_rxqs_map;
+ struct bpf_prog __rcu *xps_prog;
#endif
#ifdef CONFIG_NET_CLS_ACT
struct mini_Qdisc __rcu *miniq_egress;
@@ -2147,6 +2148,8 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
struct sk_buff *skb,
struct net_device *sb_dev);
+int dev_change_xps_fd(struct net_device *dev, int fd);
+
/* returns the headroom that the master device needs to take in account
* when forwarding to this dev
*/
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 4a8c02c..a23d241 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -167,6 +167,7 @@ enum {
IFLA_NEW_IFINDEX,
IFLA_MIN_MTU,
IFLA_MAX_MTU,
+ IFLA_XPS,
__IFLA_MAX
};
@@ -979,6 +980,17 @@ enum {
#define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
+/* XPS section */
+
+enum {
+ IFLA_XPS_UNSPEC,
+ IFLA_XPS_FD,
+ IFLA_XPS_ATTACHED,
+ __IFLA_XPS_MAX,
+};
+
+#define IFLA_XPS_MAX (__IFLA_XPS_MAX - 1)
+
enum {
IFLA_EVENT_NONE,
IFLA_EVENT_REBOOT, /* internal reset / reboot */
diff --git a/net/core/dev.c b/net/core/dev.c
index 71b18e8..a46d42b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3663,26 +3663,34 @@ static int get_xps_queue(struct net_device *dev, struct net_device *sb_dev,
{
#ifdef CONFIG_XPS
struct xps_dev_maps *dev_maps;
+ struct bpf_prog *prog;
struct sock *sk = skb->sk;
+ int bpf_ret = -1;
int queue_index = -1;
if (!static_key_false(&xps_needed))
return -1;
rcu_read_lock();
- if (!static_key_false(&xps_rxqs_needed))
- goto get_cpus_map;
- dev_maps = rcu_dereference(sb_dev->xps_rxqs_map);
- if (dev_maps) {
- int tci = sk_rx_queue_get(sk);
+ prog = rcu_dereference(dev->xps_prog);
+ if (prog) {
+ bpf_ret = bpf_prog_run_clear_cb(prog, skb);
+ if (bpf_ret >= 0)
+ queue_index = bpf_ret % dev->num_tx_queues;
+ }
- if (tci >= 0 && tci < dev->num_rx_queues)
- queue_index = __get_xps_queue_idx(dev, skb, dev_maps,
- tci);
+ if (queue_index < 0 && static_key_false(&xps_rxqs_needed)) {
+ dev_maps = rcu_dereference(sb_dev->xps_rxqs_map);
+ if (dev_maps) {
+ int tci = sk_rx_queue_get(sk);
+
+ if (tci >= 0 && tci < dev->num_rx_queues)
+ queue_index = __get_xps_queue_idx(dev, skb,
+ dev_maps, tci);
+ }
}
-get_cpus_map:
if (queue_index < 0) {
dev_maps = rcu_dereference(sb_dev->xps_cpus_map);
if (dev_maps) {
@@ -8170,6 +8178,41 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
return err;
}
+static void dev_xps_install(struct net_device *dev, struct bpf_prog *prog)
+{
+#ifdef CONFIG_XPS
+ struct bpf_prog *old = rtnl_dereference(dev->xps_prog);
+ struct bpf_prog *new = prog;
+
+ rcu_assign_pointer(dev->xps_prog, new);
+ if (old)
+ bpf_prog_put(old);
+#endif
+}
+
+/**
+ * dev_change_xps_fd - set or clear a bpf program for tx queue selection for a device
+ * @dev: device
+ * @fd: new program fd or negative value to clear
+ *
+ * Set or clear a bpf program for a device
+ */
+int dev_change_xps_fd(struct net_device *dev, int fd)
+{
+ struct bpf_prog *prog = NULL;
+
+ ASSERT_RTNL();
+
+ prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_SOCKET_FILTER);
+
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ dev_xps_install(dev, prog);
+
+ return 0;
+}
+
/**
* dev_new_index - allocate an ifindex
* @net: the applicable net namespace
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1ee6460..202b59a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -980,6 +980,15 @@ static size_t rtnl_xdp_size(void)
return xdp_size;
}
+static size_t rtnl_xps_size(void)
+{
+ size_t xps_size = nla_total_size(0) + /* nest IFLA_XPS */
+ nla_total_size(1) + /* XPS_ATTACHED */
+ nla_total_size(4); /* XPS_PROG_ID */
+
+ return xps_size;
+}
+
static noinline size_t if_nlmsg_size(const struct net_device *dev,
u32 ext_filter_mask)
{
@@ -1018,6 +1027,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
+ nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
+ nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
+ rtnl_xdp_size() /* IFLA_XDP */
+ + rtnl_xps_size() /* IFLA_XPS */
+ nla_total_size(4) /* IFLA_EVENT */
+ nla_total_size(4) /* IFLA_NEW_NETNSID */
+ nla_total_size(4) /* IFLA_NEW_IFINDEX */
@@ -1455,6 +1465,31 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
return err;
}
+static int rtnl_xps_fill(struct sk_buff *skb, struct net_device *dev)
+{
+ struct nlattr *xps;
+ struct bpf_prog *xps_prog;
+ int err;
+
+ ASSERT_RTNL();
+
+ xps = nla_nest_start(skb, IFLA_XPS);
+ if (!xps)
+ return -EMSGSIZE;
+
+ xps_prog = rtnl_dereference(dev->xps_prog);
+ if (xps_prog) {
+ err = nla_put_u8(skb, IFLA_XPS_ATTACHED, xps_prog->aux->id);
+ if (err) {
+ nla_nest_cancel(skb, xps);
+ return err;
+ }
+ }
+
+ nla_nest_end(skb, xps);
+ return 0;
+}
+
static u32 rtnl_get_event(unsigned long event)
{
u32 rtnl_event_type = IFLA_EVENT_NONE;
@@ -1697,6 +1732,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
goto nla_put_failure_rcu;
rcu_read_unlock();
+ if (rtnl_xps_fill(skb, dev))
+ goto nla_put_failure;
+
nlmsg_end(skb, nlh);
return 0;
@@ -1750,6 +1788,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
[IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
[IFLA_MIN_MTU] = { .type = NLA_U32 },
[IFLA_MAX_MTU] = { .type = NLA_U32 },
+ [IFLA_XPS] = { .type = NLA_NESTED },
};
static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -1801,6 +1840,11 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
[IFLA_XDP_PROG_ID] = { .type = NLA_U32 },
};
+static const struct nla_policy ifla_xps_policy[IFLA_XPS_MAX + 1] = {
+ [IFLA_XPS_FD] = { .type = NLA_S32 },
+ [IFLA_XPS_ATTACHED] = { .type = NLA_U8 },
+};
+
static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla)
{
const struct rtnl_link_ops *ops = NULL;
@@ -2709,6 +2753,24 @@ static int do_setlink(const struct sk_buff *skb,
}
}
+ if (tb[IFLA_XPS]) {
+ struct nlattr *xps[IFLA_XPS_MAX + 1];
+
+ err = nla_parse_nested_deprecated(xps, IFLA_XPS_MAX,
+ tb[IFLA_XPS],
+ ifla_xps_policy, NULL);
+ if (err < 0)
+ goto errout;
+
+ if (xps[IFLA_XPS_FD]) {
+ err = dev_change_xps_fd(dev,
+ nla_get_s32(xps[IFLA_XPS_FD]));
+ if (err)
+ goto errout;
+ status |= DO_SETLINK_NOTIFY;
+ }
+ }
+
errout:
if (status & DO_SETLINK_MODIFIED) {
if ((status & DO_SETLINK_NOTIFY) == DO_SETLINK_NOTIFY)
--
1.8.3.1
^ permalink raw reply related
* [RFC {net,iproute2}-next 0/2] Introduce an eBPF hookpoint for tx queue selection in the XPS (Transmit Packet Steering) code.
From: Matthew Cover @ 2019-09-19 22:44 UTC (permalink / raw)
To: davem, ast, daniel, kafai, songliubraving, yhs, nikolay, sd,
sbrivio, vincent, kda, matthew.cover, jiri, edumazet, pabeni,
idosch, petrm, f.fainelli, stephen, dsahern, christian,
jakub.kicinski, roopa, johannes.berg, mkubecek, netdev,
linux-kernel, bpf
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.
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.
Third, the TUNSETSTEERINGEBPF interface seems to require possession of existing
or creation of new queues/fds.
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).
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".
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.
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.
^ permalink raw reply
* Re: [PATCH v1 1/3] seccomp: add SECCOMP_USER_NOTIF_FLAG_CONTINUE
From: Christian Brauner @ 2019-09-19 22:22 UTC (permalink / raw)
To: Jann Horn
Cc: Kees Cook, Andy Lutomirski, Will Drewry, Shuah Khan,
Alexei Starovoitov, Daniel Borkmann, kafai, Song Liu, yhs,
kernel list, open list:KERNEL SELFTEST FRAMEWORK,
Network Development, bpf, Tycho Andersen, Tyler Hicks
In-Reply-To: <CAG48ez1QkJAMgTpqv4EqbDmYPPpxuB8cR=XhUAr1fHZOBY_DHg@mail.gmail.com>
On Thu, Sep 19, 2019 at 09:37:06PM +0200, Jann Horn wrote:
> On Thu, Sep 19, 2019 at 11:59 AM Christian Brauner
> <christian.brauner@ubuntu.com> wrote:
> > This allows the seccomp notifier to continue a syscall.
> [...]
> > Recently we landed seccomp support for SECCOMP_RET_USER_NOTIF (cf. [4])
> > which enables a process (watchee) to retrieve an fd for its seccomp
> > filter. This fd can then be handed to another (usually more privileged)
> > process (watcher). The watcher will then be able to receive seccomp
> > messages about the syscalls having been performed by the watchee.
> [...]
> > This can be solved by
> > telling seccomp to resume the syscall.
> [...]
> > @@ -780,8 +783,14 @@ static void seccomp_do_user_notification(int this_syscall,
> > list_del(&n.list);
> > out:
> > mutex_unlock(&match->notify_lock);
> > +
> > + /* Userspace requests to continue the syscall. */
> > + if (flags & SECCOMP_USER_NOTIF_FLAG_CONTINUE)
> > + return 0;
> > +
> > syscall_set_return_value(current, task_pt_regs(current),
> > err, ret);
> > + return -1;
> > }
>
> Seccomp currently expects the various seccomp return values to be
> fully ordered based on how much action the kernel should take against
> the requested syscall. Currently, the range of return values is
> basically divided into three regions: "block syscall in some way"
> (from SECCOMP_RET_KILL_PROCESS to SECCOMP_RET_USER_NOTIF), "let ptrace
> decide" (SECCOMP_RET_TRACE) and "allow" (SECCOMP_RET_LOG and
> SECCOMP_RET_ALLOW). If SECCOMP_RET_USER_NOTIF becomes able to allow
> syscalls, it will be able to override a negative decision from
> SECCOMP_RET_TRACE.
>
> In practice, that's probably not a big deal, since I'm not aware of
> anyone actually using SECCOMP_RET_TRACE for security purposes, and on
> top of that, you'd have to allow ioctl(..., SECCOMP_IOCTL_NOTIF_SEND,
> ...) and seccomp() with SECCOMP_FILTER_FLAG_NEW_LISTENER in your
> seccomp policy for this to work.
>
> More interestingly, what about the case where two
> SECCOMP_RET_USER_NOTIF filters are installed? The most recently
> installed filter takes precedence if the return values's action parts
> are the same (and this is also documented in the manpage); so if a
> container engine installs a filter that always intercepts sys_foobar()
> (and never uses SECCOMP_USER_NOTIF_FLAG_CONTINUE), and then something
> inside the container also installs a filter that always intercepts
> sys_foobar() (and always uses SECCOMP_USER_NOTIF_FLAG_CONTINUE), the
> container engine's filter will become ineffective.
Excellent point. We discussed the nested container case today.
>
> With my tendency to overcomplicate things, I'm thinking that maybe it
> might be a good idea to:
> - collect a list of all filters that returned SECCOMP_RET_USER_NOTIF,
> as well as the highest-precedence return value that was less strict
> than SECCOMP_RET_USER_NOTIF
> - sequentially send notifications to all of the
> SECCOMP_RET_USER_NOTIF filters until one doesn't return
> SECCOMP_USER_NOTIF_FLAG_CONTINUE
> - if all returned SECCOMP_USER_NOTIF_FLAG_CONTINUE, go with the
> highest-precedence return value that was less strict than
> SECCOMP_RET_USER_NOTIF, or allow if no such return value was
> encountered
>
> But perhaps, for now, it would also be enough to just expand the big
> fat warning note and tell people that if they allow the use of
> SECCOMP_IOCTL_NOTIF_SEND and SECCOMP_FILTER_FLAG_NEW_LISTENER in their
> filter, SECCOMP_RET_USER_NOTIF is bypassable. And if someone actually
> has a usecase where SECCOMP_RET_USER_NOTIF should be secure and nested
> SECCOMP_RET_USER_NOTIF support is needed, that more complicated logic
> could be added later?
Yes, I think that is the correct approach for now.
Realistically, the most useful scenario is a host-privileged supervisor
process and a user-namespaced supervised process (or to use a concrete
example, a host-privileged container manager and an unprivileged
container). Having a user-namespaced supervisor process supervising
another nested user-namespaced process is for the most part useless
because the supervisor can't do any of the interesting syscalls (e.g.
mounting block devices that are deemed safe, faking mknod() etc.). So I
expect seccomp with USER_NOTIF to be blocked just for good measure.
Also - maybe I'm wrong - the warning we added points out that this is
only safe if the supervised process can already rely on kernel (or
other) restrictions, i.e. even if an attacker overwrites pointer syscall
arguments with harmful ones the supervisor must be sure that they are
already blocked anyway. Which can be generalized to: if an unwanted
syscall goes through in _some_ way then the supervisor must be sure that
it is blocked.
Iiuc, for your specific attack all the nested attacker can do is to
never actually get the (outer) supervisor to fake the syscall for it.
A more interesting case might be where the host-privileged supervising
process wants to deny a syscall that would otherwise succeed. But if
that's the case then the outer supervisor is trying to implement a
security policy. But we explicitly point out that this is not possible
with the notifier in general.
But honestly, that is very advanced and it seems unlikely that someone
would want this. So I'd say let's just point this out.
Christian
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: mv88e6xxx: Add support for port mirroring
From: Florian Fainelli @ 2019-09-19 22:12 UTC (permalink / raw)
To: Jason Cobham, Iwan R Timmer
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <CAKu_b=+0=KXnT-b8L2qkUxT2jSMAJaiMBNAeSjJ3hPqZgx4PGw@mail.gmail.com>
On 9/19/19 2:30 PM, Jason Cobham wrote:
> Hi Iwan,
>
>> Hi Andrew,
>>
>> I only own a simple 5 ports switch (88E6176) which has no problem of mirroring the other ports to a single port. Except for a bandwith shortage ofcourse. While I thought I checked adding and removing ports, I seemed to forgot to check removing ingress traffic as it will now >disable mirroring egress traffic. Searching for how I can distinct ingress from egress mirroring in port_mirror_del, I saw there is a variable in the mirror struct called ingress. Which seems strange, because why is it a seperate argument to the port_mirror_add function?
>>
>> Origally I planned to be able to set the egress and ingress mirror seperatly. But in my laziness when I saw there already was a function to configure the destination port this functionality was lost.
>>
>> Because the other drivers which implemented the port_mirror_add (b53 and
>> ksz9477) also lacks additional checks to prevent new mirror filters from breaking previous ones I assumed they were not necessary.
>>
>> At least I will soon sent a new version with at least the issue of removing mirror ingress traffic fixed and the ability to define a seperate ingress and egress port.
>>
>> Regards,
>> Iwan
>
> I have a similar patch set for port mirror from a few years ago. I'd
> also like to see this functionality in mainline. One issue I ran into
> is when doing port mirror in a cross-chip dsa configuration. If the
> ingress and egress ports are on different chips, the ingress chip
> needs to set the egress to the cross-chip dsa port and the cross-chip
> egress port needs to be set appropriately. I also had the
> functionality to mirror egress from a port to a destination port.
>
> Is it appropriate to send my patch to the mailing list for review or
> should we work on this off-line?
Given that the net-next tree is closed at the moment, working offline
and posting a combined version of a patch that supports port mirroring
for cross chip configurations as well as standalone sounds good to me.
Thanks!
--
Florian
^ permalink raw reply
* [PATCH iproute2 master] bpf: Fix race condition with map pinning
From: Joe Stringer @ 2019-09-19 22:07 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>
---
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;
+ }
fprintf(stderr, "Could not pin %s map: %s\n", name,
strerror(errno));
close(fd);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v1 3/3] seccomp: test SECCOMP_USER_NOTIF_FLAG_CONTINUE
From: Christian Brauner @ 2019-09-19 21:50 UTC (permalink / raw)
To: shuah
Cc: keescook, luto, jannh, wad, ast, daniel, kafai, songliubraving,
yhs, linux-kernel, linux-kselftest, netdev, bpf, Tycho Andersen,
Tyler Hicks, stable
In-Reply-To: <ad7d2901-6639-3684-b71c-bdc1a6a020cc@kernel.org>
On Thu, Sep 19, 2019 at 11:13:46AM -0600, shuah wrote:
> On 9/19/19 3:59 AM, Christian Brauner wrote:
> > Test whether a syscall can be performed after having been intercepted by
> > the seccomp notifier. The test uses dup() and kcmp() since it allows us to
> > nicely test whether the dup() syscall actually succeeded by comparing whether
> > the fds refer to the same underlying struct file.
> >
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Will Drewry <wad@chromium.org>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Cc: Martin KaFai Lau <kafai@fb.com>
> > Cc: Song Liu <songliubraving@fb.com>
> > Cc: Yonghong Song <yhs@fb.com>
> > Cc: Tycho Andersen <tycho@tycho.ws>
> > CC: Tyler Hicks <tyhicks@canonical.com>
> > Cc: stable@vger.kernel.org
> > Cc: linux-kselftest@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Cc: bpf@vger.kernel.org
> > ---
> > /* v1 */
> > - Christian Brauner <christian.brauner@ubuntu.com>:
> > - adapt to new flag name SECCOMP_USER_NOTIF_FLAG_CONTINUE
> >
> > /* v0 */
> > Link: https://lore.kernel.org/r/20190918084833.9369-5-christian.brauner@ubuntu.com
> > ---
> > tools/testing/selftests/seccomp/seccomp_bpf.c | 102 ++++++++++++++++++
> > 1 file changed, 102 insertions(+)
> >
> > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > index e996d7b7fd6e..b0966599acb5 100644
> > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > @@ -44,6 +44,7 @@
> > #include <sys/times.h>
> > #include <sys/socket.h>
> > #include <sys/ioctl.h>
> > +#include <linux/kcmp.h>
> > #include <unistd.h>
> > #include <sys/syscall.h>
> > @@ -167,6 +168,10 @@ struct seccomp_metadata {
> > #define SECCOMP_RET_USER_NOTIF 0x7fc00000U
> > +#ifndef SECCOMP_USER_NOTIF_FLAG_CONTINUE
> > +#define SECCOMP_USER_NOTIF_FLAG_CONTINUE 0x00000001
> > +#endif
> > +
> > #define SECCOMP_IOC_MAGIC '!'
> > #define SECCOMP_IO(nr) _IO(SECCOMP_IOC_MAGIC, nr)
> > #define SECCOMP_IOR(nr, type) _IOR(SECCOMP_IOC_MAGIC, nr, type)
> > @@ -3481,6 +3486,103 @@ TEST(seccomp_get_notif_sizes)
> > EXPECT_EQ(sizes.seccomp_notif_resp, sizeof(struct seccomp_notif_resp));
> > }
> > +static int filecmp(pid_t pid1, pid_t pid2, int fd1, int fd2)
> > +{
> > +#ifdef __NR_kcmp
> > + return syscall(__NR_kcmp, pid1, pid2, KCMP_FILE, fd1, fd2);
> > +#else
> > + errno = ENOSYS;
> > + return -1;
>
> This should be SKIP for kselftest so this isn't counted a failure.
> In this case test can't be run because of a missing dependency.
Right, I can just ifdef the whole test and report a skip.
^ permalink raw reply
* [PATCH RFC] thermal: Fix broken registration if a sensor OF node is missing
From: Peter Mamonov @ 2019-09-19 21:40 UTC (permalink / raw)
To: rui.zhang, edubezval, daniel.lezcano
Cc: andrew, davem, netdev, linux-pm, Peter Mamonov, stable
When devm_thermal_zone_of_sensor_register() is called from
hwmon_thermal_add_sensor() it is possible that the relevant sensor is
missing an OF node. In this case thermal_zone_of_sensor_register() returns
-EINVAL which causes hwmon_thermal_add_sensor() to fail as well. This patch
changes relevant return code of thermal_zone_of_sensor_register() to
-ENODEV, which is tolerated by hwmon_thermal_add_sensor().
Here is a particular case of such behaviour: the Marvell ethernet PHYs
driver registers hwmon device for the built-in temperature sensor (see
drivers/net/phy/marvell.c). Since the sensor doesn't have associated OF
node devm_hwmon_device_register() returns error which ultimately causes
failure of the PHY driver's probe function.
Fixes: 4e5e4705bf69 ("thermal: introduce device tree parser")
Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Cc: stable@vger.kernel.org
---
drivers/thermal/of-thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index dc5093be553e..34b0cc173f4a 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -493,7 +493,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
if (!dev || !dev->of_node) {
of_node_put(np);
- return ERR_PTR(-EINVAL);
+ return ERR_PTR(-ENODEV);
}
sensor_np = of_node_get(dev->of_node);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH net-next] net: dsa: mv88e6xxx: Add support for port mirroring
From: Jason Cobham @ 2019-09-19 21:30 UTC (permalink / raw)
To: Iwan R Timmer
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev
Hi Iwan,
>Hi Andrew,
>
>I only own a simple 5 ports switch (88E6176) which has no problem of mirroring the other ports to a single port. Except for a bandwith shortage ofcourse. While I thought I checked adding and removing ports, I seemed to forgot to check removing ingress traffic as it will now >disable mirroring egress traffic. Searching for how I can distinct ingress from egress mirroring in port_mirror_del, I saw there is a variable in the mirror struct called ingress. Which seems strange, because why is it a seperate argument to the port_mirror_add function?
>
>Origally I planned to be able to set the egress and ingress mirror seperatly. But in my laziness when I saw there already was a function to configure the destination port this functionality was lost.
>
>Because the other drivers which implemented the port_mirror_add (b53 and
>ksz9477) also lacks additional checks to prevent new mirror filters from breaking previous ones I assumed they were not necessary.
>
>At least I will soon sent a new version with at least the issue of removing mirror ingress traffic fixed and the ability to define a seperate ingress and egress port.
>
>Regards,
>Iwan
I have a similar patch set for port mirror from a few years ago. I'd
also like to see this functionality in mainline. One issue I ran into
is when doing port mirror in a cross-chip dsa configuration. If the
ingress and egress ports are on different chips, the ingress chip
needs to set the egress to the cross-chip dsa port and the cross-chip
egress port needs to be set appropriately. I also had the
functionality to mirror egress from a port to a destination port.
Is it appropriate to send my patch to the mailing list for review or
should we work on this off-line?
Thanks,
Jason
^ permalink raw reply
* Re: mt76x2e hardware restart
From: Oleksandr Natalenko @ 2019-09-19 21:22 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: <deaafa7a3e9ea2111ebb5106430849c6@natalenko.name>
On 19.09.2019 18:24, Oleksandr Natalenko wrote:
> [ +9,979664] mt76x2e 0000:01:00.0: Firmware Version: 0.0.00
> [ +0,000014] mt76x2e 0000:01:00.0: Build: 1
> [ +0,000010] mt76x2e 0000:01:00.0: Build Time: 201507311614____
> [ +0,018017] mt76x2e 0000:01:00.0: Firmware running!
> [ +0,001101] ieee80211 phy4: Hardware restart was requested
IIUC, this happens due to watchdog. I think the following applies.
Watchdog is started here:
=== mt76x02_util.c
130 void mt76x02_init_device(struct mt76x02_dev *dev)
131 {
...
155 INIT_DELAYED_WORK(&dev->wdt_work, mt76x02_wdt_work);
===
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);
===
Conditions:
=== mt76x02_mmio.c
530 static void mt76x02_check_tx_hang(struct mt76x02_dev *dev)
531 {
532 if (mt76x02_tx_hang(dev)) {
533 if (++dev->tx_hang_check >= MT_TX_HANG_TH)
534 goto restart;
535 } else {
536 dev->tx_hang_check = 0;
537 }
538
539 if (dev->mcu_timeout)
540 goto restart;
541
542 return;
543
544 restart:
545 mt76x02_watchdog_reset(dev);
===
Actual check:
=== mt76x02_mmio.c
367 static bool mt76x02_tx_hang(struct mt76x02_dev *dev)
368 {
369 u32 dma_idx, prev_dma_idx;
370 struct mt76_queue *q;
371 int i;
372
373 for (i = 0; i < 4; i++) {
374 q = dev->mt76.q_tx[i].q;
375
376 if (!q->queued)
377 continue;
378
379 prev_dma_idx = dev->mt76.tx_dma_idx[i];
380 dma_idx = readl(&q->regs->dma_idx);
381 dev->mt76.tx_dma_idx[i] = dma_idx;
382
383 if (prev_dma_idx == dma_idx)
384 break;
385 }
386
387 return i < 4;
388 }
===
(I don't quite understand what it does here; why 4? does each device
have 4 queues? maybe, my does not? I guess this is where watchdog is
triggered, though, because otherwise I'd see mcu_timeout message like
"MCU message %d (seq %d) timed out\n")
Once it detects TX hang, the reset is triggered:
=== mt76x02_mmio.c
446 static void mt76x02_watchdog_reset(struct mt76x02_dev *dev)
447 {
...
485 if (restart)
486 mt76_mcu_restart(dev);
===
mt76_mcu_restart() is just a define for this series here:
=== mt76.h
555 #define mt76_mcu_restart(dev, ...)
(dev)->mt76.mcu_ops->mcu_restart(&((dev)->mt76))
===
Actual OP:
=== mt76x2/pci_mcu.c
188 int mt76x2_mcu_init(struct mt76x02_dev *dev)
189 {
190 static const struct mt76_mcu_ops mt76x2_mcu_ops = {
191 .mcu_restart = mt76pci_mcu_restart,
192 .mcu_send_msg = mt76x02_mcu_msg_send,
193 };
===
This triggers loading the firmware:
=== mt76x2/pci_mcu.c
168 static int
169 mt76pci_mcu_restart(struct mt76_dev *mdev)
170 {
...
179 ret = mt76pci_load_firmware(dev);
===
which does the printout I observe:
=== mt76x2/pci_mcu.c
91 static int
92 mt76pci_load_firmware(struct mt76x02_dev *dev)
93 {
...
156 dev_info(dev->mt76.dev, "Firmware running!\n");
===
Too bad it doesn't show the actual watchdog message, IOW, why the reset
happens. I guess I will have to insert some pr_infos here and there.
Does it make sense? Any ideas why this can happen?
More info on the device during boot:
===
[ +0,333233] mt76x2e 0000:01:00.0: enabling device (0000 -> 0002)
[ +0,000571] mt76x2e 0000:01:00.0: ASIC revision: 76120044
[ +0,017806] mt76x2e 0000:01:00.0: ROM patch build: 20141115060606a
===
--
Oleksandr Natalenko (post-factum)
^ permalink raw reply
* Re: Bug report (with fix) for DEC Tulip driver (de2104x.c)
From: Sven Schnelle @ 2019-09-19 20:31 UTC (permalink / raw)
To: Helge Deller
Cc: John David Anglin, Arlie Davis, Andrew Lunn, netdev, linux-parisc
In-Reply-To: <f71e9773-5cfb-f20b-956f-d98b11a5d4a7@gmx.de>
Hi,
On Wed, Sep 18, 2019 at 07:56:16AM +0200, Helge Deller wrote:
> On 18.09.19 00:51, John David Anglin wrote:
> > On 2019-09-17 5:36 p.m., Arlie Davis wrote:
> >> Likewise, I'm at a loss for testing with real hardware. It's hard to
> >> find such things, now.
> > How does de2104x compare to ds2142/43? I have a c3750 with ds2142/43 tulip. Helge
> > or some others might have a machine with a de2104x.
>
> The machines we could test are
> * a C240 with a DS21140 tulip chip (Sven has one),
My C240 identifies as C240+:
[ 0.000000] model 9000/782/C240+
which has a 21143 (verified by looking at the board):
root@c240:/# lspci -kvvnn -s 00:14.0
00:14.0 Ethernet controller [0200]: Digital Equipment Corporation DECchip 21142/43 [1011:0019] (rev 30)
Subsystem: Hewlett-Packard Company DECchip 21142/43 [103c:104f]
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx-
Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 255 (5000ns min, 10000ns max), Cache Line Size: 32 bytes
Interrupt: pin A routed to IRQ 23
Region 0: I/O ports at 0080 [size=128]
Region 1: Memory at f2802000 (32-bit, non-prefetchable) [size=128]
Expansion ROM at f2f80000 [disabled] [size=256K]
Kernel driver in use: tulip
Regards
Sven
^ permalink raw reply
* Re: [PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
From: Vincent Cheng @ 2019-09-19 20:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh+dt@kernel.org, mark.rutland@arm.com,
richardcochran@gmail.com, devicetree@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190918211803.GO9591@lunn.ch>
Hi Andrew,
On Wed, Sep 18, 2019 at 05:18:03PM EDT, Andrew Lunn wrote:
>On Wed, Sep 18, 2019 at 04:06:38PM -0400, vincent.cheng.xh@renesas.com wrote:
>
>> +static s32 idtcm_xfer(struct idtcm *idtcm,
>> + u8 regaddr,
>> + u8 *buf,
>> + u16 count,
>> + bool write)
>> +{
>> + struct i2c_client *client = idtcm->client;
>> + struct i2c_msg msg[2];
>> + s32 cnt;
>> +
>> + msg[0].addr = client->addr;
>> + msg[0].flags = 0;
>> + msg[0].len = 1;
>> + msg[0].buf = ®addr;
>> +
>> + msg[1].addr = client->addr;
>> + msg[1].flags = write ? 0 : I2C_M_RD;
>> + msg[1].len = count;
>> + msg[1].buf = buf;
>> +
>> + cnt = i2c_transfer(client->adapter, msg, 2);
>> +
>> + if (cnt < 0) {
>> + pr_err("i2c_transfer returned %d\n", cnt);
>
>dev_err(client->dev, "i2c_transfer returned %d\n", cnt);
>
>We then have an idea which device has a transfer error.
>
>Please try to not use pr_err() when you have some sort of device.
Sure thing, will replace pr_err() with dev_err().
>> +static s32 idtcm_state_machine_reset(struct idtcm *idtcm)
>> +{
>> + s32 err;
>> + u8 byte = SM_RESET_CMD;
>> +
>> + err = idtcm_write(idtcm, RESET_CTRL, SM_RESET, &byte, sizeof(byte));
>> +
>> + if (!err) {
>> + /* delay */
>> + set_current_state(TASK_INTERRUPTIBLE);
>> + schedule_timeout(_msecs_to_jiffies(POST_SM_RESET_DELAY_MS));
>
>Maybe use msleep_interruptable()?
Yes, will try using msleep_interruptable() and will replace if it works.
>> +static s32 idtcm_load_firmware(struct idtcm *idtcm,
>> + struct device *dev)
>> +{
>> + const struct firmware *fw;
>> + struct idtcm_fwrc *rec;
>> + u32 regaddr;
>> + s32 err;
>> + s32 len;
>> + u8 val;
>> + u8 loaddr;
>> +
>> + pr_info("requesting firmware '%s'\n", FW_FILENAME);
>
>dev_debug()
Thanks, will make the change.
>> +
>> + err = request_firmware(&fw, FW_FILENAME, dev);
>> +
>> + if (err)
>> + return err;
>> +
>> + pr_info("firmware size %zu bytes\n", fw->size);
>
>dev_debug()
>
>Maybe look through all your pr_info and downgrade most of them to
>dev_debug()
Yes, will go through and downgrade to dev_debug() accordingly.
Thanks,
Vincent
^ 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