Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH V2 6/8] mdev: introduce virtio device and its device ops
From: Jason Wang @ 2019-09-25 12:06 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm, linux-s390, linux-kernel, dri-devel, intel-gfx,
	intel-gvt-dev, kwankhede, mst, tiwei.bie, virtualization, netdev,
	cohuck, maxime.coquelin, cunming.liang, zhihong.wang, rob.miller,
	xiao.w.wang, haotian.wang, zhenyuw, zhi.a.wang, jani.nikula,
	joonas.lahtinen, rodrigo.vivi, airlied, daniel, farman, pasic,
	sebott, oberpar, heiko.carstens, gor, borntraeger, akrowiak,
	freude, lingshan.zhu, idos, eperezma, lulu, parav,
	christophe.de.dinechin, kevin.tian
In-Reply-To: <20190924170640.1da03bae@x1.home>


On 2019/9/25 上午7:06, Alex Williamson wrote:
> On Tue, 24 Sep 2019 21:53:30 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> This patch implements basic support for mdev driver that supports
>> virtio transport for kernel virtio driver.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  include/linux/mdev.h        |   2 +
>>  include/linux/virtio_mdev.h | 145 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 147 insertions(+)
>>  create mode 100644 include/linux/virtio_mdev.h
>>
>> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
>> index 3414307311f1..73ac27b3b868 100644
>> --- a/include/linux/mdev.h
>> +++ b/include/linux/mdev.h
>> @@ -126,6 +126,8 @@ struct mdev_device *mdev_from_dev(struct device *dev);
>>  
>>  enum {
>>  	MDEV_ID_VFIO = 1,
>> +	MDEV_ID_VIRTIO = 2,
>> +	MDEV_ID_VHOST = 3,
> MDEV_ID_VHOST isn't used yet here. 


Yes, just want to reserve one but it look too early to do that.


>  Also, given the strong
> interdependence between the class_id and the ops structure, we might
> wand to define them in the same place.  Thanks,
>
> Alex


Yes, as you suggest in patch 5/8.

Thanks


>
>>  	/* New entries must be added here */
>>  };
>>  
>> diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h
>> new file mode 100644
>> index 000000000000..d1a40a739266
>> --- /dev/null
>> +++ b/include/linux/virtio_mdev.h
>> @@ -0,0 +1,145 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Virtio mediated device driver
>> + *
>> + * Copyright 2019, Red Hat Corp.
>> + *     Author: Jason Wang <jasowang@redhat.com>
>> + */
>> +#ifndef _LINUX_VIRTIO_MDEV_H
>> +#define _LINUX_VIRTIO_MDEV_H
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/mdev.h>
>> +#include <uapi/linux/vhost.h>
>> +
>> +#define VIRTIO_MDEV_DEVICE_API_STRING		"virtio-mdev"
>> +#define VIRTIO_MDEV_VERSION 0x1
>> +
>> +struct virtio_mdev_callback {
>> +	irqreturn_t (*callback)(void *data);
>> +	void *private;
>> +};
>> +
>> +/**
>> + * struct vfio_mdev_device_ops - Structure to be registered for each
>> + * mdev device to register the device to virtio-mdev module.
>> + *
>> + * @set_vq_address:		Set the address of virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@desc_area: address of desc area
>> + *				@driver_area: address of driver area
>> + *				@device_area: address of device area
>> + *				Returns integer: success (0) or error (< 0)
>> + * @set_vq_num:		Set the size of virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@num: the size of virtqueue
>> + * @kick_vq:			Kick the virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + * @set_vq_cb:			Set the interrut calback function for
>> + *				a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@cb: virtio-mdev interrupt callback structure
>> + * @set_vq_ready:		Set ready status for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@ready: ready (true) not ready(false)
>> + * @get_vq_ready:		Get ready status for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				Returns boolean: ready (true) or not (false)
>> + * @set_vq_state:		Set the state for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@state: virtqueue state (last_avail_idx)
>> + *				Returns integer: success (0) or error (< 0)
>> + * @get_vq_state:		Get the state for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				Returns virtqueue state (last_avail_idx)
>> + * @get_vq_align:		Get the virtqueue align requirement
>> + *				for the device
>> + *				@mdev: mediated device
>> + *				Returns virtqueue algin requirement
>> + * @get_features:		Get virtio features supported by the device
>> + *				@mdev: mediated device
>> + *				Returns the features support by the
>> + *				device
>> + * @get_features:		Set virtio features supported by the driver
>> + *				@mdev: mediated device
>> + *				@features: feature support by the driver
>> + *				Returns integer: success (0) or error (< 0)
>> + * @set_config_cb:		Set the config interrupt callback
>> + *				@mdev: mediated device
>> + *				@cb: virtio-mdev interrupt callback structure
>> + * @get_device_id:		Get virtio device id
>> + *				@mdev: mediated device
>> + *				Returns u32: virtio device id
>> + * @get_vendor_id:		Get virtio vendor id
>> + *				@mdev: mediated device
>> + *				Returns u32: virtio vendor id
>> + * @get_status:		Get the device status
>> + *				@mdev: mediated device
>> + *				Returns u8: virtio device status
>> + * @set_status:		Set the device status
>> + *				@mdev: mediated device
>> + *				@status: virtio device status
>> + * @get_config:		Read from device specific confiugration space
>> + *				@mdev: mediated device
>> + *				@offset: offset from the beginning of
>> + *				configuration space
>> + *				@buf: buffer used to read to
>> + *				@len: the length to read from
>> + *				configration space
>> + * @set_config:		Write to device specific confiugration space
>> + *				@mdev: mediated device
>> + *				@offset: offset from the beginning of
>> + *				configuration space
>> + *				@buf: buffer used to write from
>> + *				@len: the length to write to
>> + *				configration space
>> + * @get_version:		Get the version of virtio mdev device
>> + *				@mdev: mediated device
>> + *				Returns integer: version of the device
>> + * @get_generation:		Get device generaton
>> + *				@mdev: mediated device
>> + *				Returns u32: device generation
>> + */
>> +struct virtio_mdev_device_ops {
>> +	/* Virtqueue ops */
>> +	int (*set_vq_address)(struct mdev_device *mdev,
>> +			      u16 idx, u64 desc_area, u64 driver_area,
>> +			      u64 device_area);
>> +	void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num);
>> +	void (*kick_vq)(struct mdev_device *mdev, u16 idx);
>> +	void (*set_vq_cb)(struct mdev_device *mdev, u16 idx,
>> +			  struct virtio_mdev_callback *cb);
>> +	void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool ready);
>> +	bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx);
>> +	int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state);
>> +	u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx);
>> +
>> +	/* Device ops */
>> +	u16 (*get_vq_align)(struct mdev_device *mdev);
>> +	u64 (*get_features)(struct mdev_device *mdev);
>> +	int (*set_features)(struct mdev_device *mdev, u64 features);
>> +	void (*set_config_cb)(struct mdev_device *mdev,
>> +			      struct virtio_mdev_callback *cb);
>> +	u16 (*get_queue_max)(struct mdev_device *mdev);
>> +	u32 (*get_device_id)(struct mdev_device *mdev);
>> +	u32 (*get_vendor_id)(struct mdev_device *mdev);
>> +	u8 (*get_status)(struct mdev_device *mdev);
>> +	void (*set_status)(struct mdev_device *mdev, u8 status);
>> +	void (*get_config)(struct mdev_device *mdev, unsigned int offset,
>> +			   void *buf, unsigned int len);
>> +	void (*set_config)(struct mdev_device *mdev, unsigned int offset,
>> +			   const void *buf, unsigned int len);
>> +	int (*get_version)(struct mdev_device *mdev);
>> +	u32 (*get_generation)(struct mdev_device *mdev);
>> +};
>> +
>> +#endif
>> +

^ permalink raw reply

* Re: [PATCH net v2 0/3] net/smc: move some definitions to UAPI
From: Ursula Braun @ 2019-09-25 12:10 UTC (permalink / raw)
  To: David Miller, esyr; +Cc: netdev, linux-kernel, kgraul
In-Reply-To: <20190924.165240.1617972512581218831.davem@davemloft.net>



On 9/24/19 4:52 PM, David Miller wrote:
> From: Eugene Syromiatnikov <esyr@redhat.com>
> Date: Fri, 20 Sep 2019 17:41:47 +0200
> 
>> As of now, it's a bit difficult to use SMC protocol, as significant part
>> of definitions related to it are defined in private headers and are not
>> part of UAPI. The following commits move some definitions to UAPI,
>> making them readily available to the user space.
>>
>> Changes since v1[1]:
>>  * Patch "provide fallback diagnostic codes in UAPI" is updated
>>    in accordance with the updated set of diagnostic codes.
>>
>> [1] https://lkml.org/lkml/2018/10/7/177
> 
> Isn't it way too late for this?
> 
> These definitions will now be duplicates for userland code that
> defines the values on their own.
>

Dave,
we have to admit that it is already late for these patches. Nevertheless
we think it is better to come up with them now than never. We doubt there
exists already much userland code for it - except our own IBM-provided
package smc-tools. Thus we appreciate acceptance of these patches.

Kind regards, Ursula

 


^ permalink raw reply

* Re: [PATCH V2 0/8] mdev based hardware virtio offloading support
From: Jason Wang @ 2019-09-25 12:13 UTC (permalink / raw)
  To: Tian, Kevin, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
	alex.williamson@redhat.com, mst@redhat.com, Bie, Tiwei
  Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
	cohuck@redhat.com, maxime.coquelin@redhat.com, Liang, Cunming,
	Wang, Zhihong, rob.miller@broadcom.com, Wang, Xiao W,
	haotian.wang@sifive.com, zhenyuw@linux.intel.com, Wang, Zhi A,
	jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
	Vivi, Rodrigo, airlied@linux.ie, daniel@ffwll.ch,
	farman@linux.ibm.com, pasic@linux.ibm.com, sebott@linux.ibm.com,
	oberpar@linux.ibm.com, heiko.carstens@de.ibm.com,
	gor@linux.ibm.com, borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
	freude@linux.ibm.com, Zhu, Lingshan, idos@mellanox.com,
	eperezma@redhat.com, lulu@redhat.com, parav@mellanox.com,
	christophe.de.dinechin@gmail.com
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D19D58F68D@SHSMSX104.ccr.corp.intel.com>


On 2019/9/25 下午4:24, Tian, Kevin wrote:
>> From: Jason Wang [mailto:jasowang@redhat.com]
>> Sent: Tuesday, September 24, 2019 9:53 PM
>>
>> Hi all:
>>
>> There are hardware that can do virtio datapath offloading while having
>> its own control path. This path tries to implement a mdev based
>> unified API to support using kernel virtio driver to drive those
>> devices. This is done by introducing a new mdev transport for virtio
>> (virtio_mdev) and register itself as a new kind of mdev driver. Then
>> it provides a unified way for kernel virtio driver to talk with mdev
>> device implementation.
>>
>> Though the series only contains kernel driver support, the goal is to
>> make the transport generic enough to support userspace drivers. This
>> means vhost-mdev[1] could be built on top as well by resuing the
>> transport.
>>
>> A sample driver is also implemented which simulate a virito-net
>> loopback ethernet device on top of vringh + workqueue. This could be
>> used as a reference implementation for real hardware driver.
>>
>> Consider mdev framework only support VFIO device and driver right now,
>> this series also extend it to support other types. This is done
>> through introducing class id to the device and pairing it with
>> id_talbe claimed by the driver. On top, this seris also decouple
> id_table claimed ... this series ...


Let me fix in V3.

Thanks


>
>> device specific parents ops out of the common ones.
>>
>> Pktgen test was done with virito-net + mvnet loop back device.
>>
>> Please review.
>>
>> [1] https://lkml.org/lkml/2019/9/16/869
>>
>> Changes from V1:
>>
>> - move virtio_mdev.c to drivers/virtio
>> - store class_id in mdev_device instead of mdev_parent
>> - store device_ops in mdev_device instead of mdev_parent
>> - reorder the patch, vringh fix comes first
>> - really silent compiling warnings
>> - really switch to use u16 for class_id
>> - uevent and modpost support for mdev class_id
>> - vraious tweaks per comments from Parav
>>
>> Changes from RFC-V2:
>>
>> - silent compile warnings on some specific configuration
>> - use u16 instead u8 for class id
>> - reseve MDEV_ID_VHOST for future vhost-mdev work
>> - introduce "virtio" type for mvnet and make "vhost" type for future
>>   work
>> - add entries in MAINTAINER
>> - tweak and typos fixes in commit log
>>
>> Changes from RFC-V1:
>>
>> - rename device id to class id
>> - add docs for class id and device specific ops (device_ops)
>> - split device_ops into seperate headers
>> - drop the mdev_set_dma_ops()
>> - use device_ops to implement the transport API, then it's not a part
>>   of UAPI any more
>> - use GFP_ATOMIC in mvnet sample device and other tweaks
>> - set_vring_base/get_vring_base support for mvnet device
>>
>> Jason Wang (8):
>>   vringh: fix copy direction of vringh_iov_push_kern()
>>   mdev: class id support
>>   mdev: bus uevent support
>>   modpost: add support for mdev class id
>>   mdev: introduce device specific ops
>>   mdev: introduce virtio device and its device ops
>>   virtio: introduce a mdev based transport
>>   docs: sample driver to demonstrate how to implement virtio-mdev
>>     framework
>>
>>  .../driver-api/vfio-mediated-device.rst       |   7 +-
>>  MAINTAINERS                                   |   2 +
>>  drivers/gpu/drm/i915/gvt/kvmgt.c              |  18 +-
>>  drivers/s390/cio/vfio_ccw_ops.c               |  18 +-
>>  drivers/s390/crypto/vfio_ap_ops.c             |  14 +-
>>  drivers/vfio/mdev/mdev_core.c                 |  19 +
>>  drivers/vfio/mdev/mdev_driver.c               |  22 +
>>  drivers/vfio/mdev/mdev_private.h              |   2 +
>>  drivers/vfio/mdev/vfio_mdev.c                 |  45 +-
>>  drivers/vhost/vringh.c                        |   8 +-
>>  drivers/virtio/Kconfig                        |   7 +
>>  drivers/virtio/Makefile                       |   1 +
>>  drivers/virtio/virtio_mdev.c                  | 417 +++++++++++
>>  include/linux/mdev.h                          |  52 +-
>>  include/linux/mod_devicetable.h               |   8 +
>>  include/linux/vfio_mdev.h                     |  52 ++
>>  include/linux/virtio_mdev.h                   | 145 ++++
>>  samples/Kconfig                               |   7 +
>>  samples/vfio-mdev/Makefile                    |   1 +
>>  samples/vfio-mdev/mbochs.c                    |  20 +-
>>  samples/vfio-mdev/mdpy.c                      |  20 +-
>>  samples/vfio-mdev/mtty.c                      |  18 +-
>>  samples/vfio-mdev/mvnet.c                     | 692 ++++++++++++++++++
>>  scripts/mod/devicetable-offsets.c             |   3 +
>>  scripts/mod/file2alias.c                      |  10 +
>>  25 files changed, 1524 insertions(+), 84 deletions(-)
>>  create mode 100644 drivers/virtio/virtio_mdev.c
>>  create mode 100644 include/linux/vfio_mdev.h
>>  create mode 100644 include/linux/virtio_mdev.h
>>  create mode 100644 samples/vfio-mdev/mvnet.c
>>
>> --
>> 2.19.1

^ permalink raw reply

* Re: [PATCH net] net: axienet: fix a signedness bug in probe
From: Andrew Lunn @ 2019-09-25 12:14 UTC (permalink / raw)
  To: David Miller
  Cc: alvaro.gamez, dan.carpenter, radhey.shyam.pandey, michal.simek,
	linux, netdev, kernel-janitors
In-Reply-To: <20190925.133507.2083224833639646147.davem@davemloft.net>

On Wed, Sep 25, 2019 at 01:35:07PM +0200, David Miller wrote:
> From: "Alvaro G. M" <alvaro.gamez@hazent.com>
> Date: Wed, 25 Sep 2019 13:05:43 +0200
> 
> > Hi, Dan
> > 
> > On Wed, Sep 25, 2019 at 01:59:11PM +0300, Dan Carpenter wrote:
> >> The "lp->phy_mode" is an enum but in this context GCC treats it as an
> >> unsigned int so the error handling is never triggered.
> >> 
> >>  		lp->phy_mode = of_get_phy_mode(pdev->dev.of_node);
> >> -		if (lp->phy_mode < 0) {
> >> +		if ((int)lp->phy_mode < 0) {
> > 
> > This (almost) exact code appears in a lot of different drivers too,
> > so maybe it'd be nice to review them all and apply the same cast if needed?
> 
> Or make the thing an int if negative values are never valid 32-bit phy_mode
> values anyways.

Maybe we should change the API

int of_get_phy_mode(struct device_node *np, phy_interface_t *phy_mode);

Separate the error from the value we are getting.

	 Andrew

^ permalink raw reply

* Re: [PATCH V2 2/8] mdev: class id support
From: Jason Wang @ 2019-09-25 12:13 UTC (permalink / raw)
  To: Tian, Kevin, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
	alex.williamson@redhat.com, mst@redhat.com, Bie, Tiwei
  Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
	cohuck@redhat.com, maxime.coquelin@redhat.com, Liang, Cunming,
	Wang, Zhihong, rob.miller@broadcom.com, Wang, Xiao W,
	haotian.wang@sifive.com, zhenyuw@linux.intel.com, Wang, Zhi A,
	jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
	Vivi, Rodrigo, airlied@linux.ie, daniel@ffwll.ch,
	farman@linux.ibm.com, pasic@linux.ibm.com, sebott@linux.ibm.com,
	oberpar@linux.ibm.com, heiko.carstens@de.ibm.com,
	gor@linux.ibm.com, borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
	freude@linux.ibm.com, Zhu, Lingshan, idos@mellanox.com,
	eperezma@redhat.com, lulu@redhat.com, parav@mellanox.com,
	christophe.de.dinechin@gmail.com
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D19D58F6AE@SHSMSX104.ccr.corp.intel.com>


On 2019/9/25 下午4:28, Tian, Kevin wrote:
>> From: Jason Wang
>> Sent: Tuesday, September 24, 2019 9:53 PM
>>
>> Mdev bus only supports vfio driver right now, so it doesn't implement
>> match method. But in the future, we may add drivers other than vfio,
>> the first driver could be virtio-mdev. This means we need to add
>> device class id support in bus match method to pair the mdev device
>> and mdev driver correctly.
>>
>> So this patch adds id_table to mdev_driver and class_id for mdev
>> parent with the match method for mdev bus.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  Documentation/driver-api/vfio-mediated-device.rst |  3 +++
>>  drivers/gpu/drm/i915/gvt/kvmgt.c                  |  1 +
>>  drivers/s390/cio/vfio_ccw_ops.c                   |  1 +
>>  drivers/s390/crypto/vfio_ap_ops.c                 |  1 +
>>  drivers/vfio/mdev/mdev_core.c                     |  7 +++++++
>>  drivers/vfio/mdev/mdev_driver.c                   | 14 ++++++++++++++
>>  drivers/vfio/mdev/mdev_private.h                  |  1 +
>>  drivers/vfio/mdev/vfio_mdev.c                     |  6 ++++++
>>  include/linux/mdev.h                              |  8 ++++++++
>>  include/linux/mod_devicetable.h                   |  8 ++++++++
>>  samples/vfio-mdev/mbochs.c                        |  1 +
>>  samples/vfio-mdev/mdpy.c                          |  1 +
>>  samples/vfio-mdev/mtty.c                          |  1 +
>>  13 files changed, 53 insertions(+)
>>
>> diff --git a/Documentation/driver-api/vfio-mediated-device.rst
>> b/Documentation/driver-api/vfio-mediated-device.rst
>> index 25eb7d5b834b..a5bdc60d62a1 100644
>> --- a/Documentation/driver-api/vfio-mediated-device.rst
>> +++ b/Documentation/driver-api/vfio-mediated-device.rst
>> @@ -102,12 +102,14 @@ structure to represent a mediated device's
>> driver::
>>        * @probe: called when new device created
>>        * @remove: called when device removed
>>        * @driver: device driver structure
>> +      * @id_table: the ids serviced by this driver
>>        */
>>       struct mdev_driver {
>>  	     const char *name;
>>  	     int  (*probe)  (struct device *dev);
>>  	     void (*remove) (struct device *dev);
>>  	     struct device_driver    driver;
>> +	     const struct mdev_class_id *id_table;
>>       };
>>
>>  A mediated bus driver for mdev should use this structure in the function
>> calls
>> @@ -165,6 +167,7 @@ register itself with the mdev core driver::
>>  	extern int  mdev_register_device(struct device *dev,
>>  	                                 const struct mdev_parent_ops *ops);
>>
>> +
>>  However, the mdev_parent_ops structure is not required in the function
>> call
>>  that a driver should use to unregister itself with the mdev core driver::
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 23aa3e50cbf8..f793252a3d2a 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -678,6 +678,7 @@ static int intel_vgpu_create(struct kobject *kobj,
>> struct mdev_device *mdev)
>>  		     dev_name(mdev_dev(mdev)));
>>  	ret = 0;
>>
>> +	mdev_set_class_id(mdev, MDEV_ID_VFIO);
>>  out:
>>  	return ret;
>>  }
>> diff --git a/drivers/s390/cio/vfio_ccw_ops.c
>> b/drivers/s390/cio/vfio_ccw_ops.c
>> index f0d71ab77c50..d258ef1fedb9 100644
>> --- a/drivers/s390/cio/vfio_ccw_ops.c
>> +++ b/drivers/s390/cio/vfio_ccw_ops.c
>> @@ -129,6 +129,7 @@ static int vfio_ccw_mdev_create(struct kobject
>> *kobj, struct mdev_device *mdev)
>>  			   private->sch->schid.ssid,
>>  			   private->sch->schid.sch_no);
>>
>> +	mdev_set_class_id(mdev, MDEV_ID_VFIO);
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
>> b/drivers/s390/crypto/vfio_ap_ops.c
>> index 5c0f53c6dde7..2cfd96112aa0 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -343,6 +343,7 @@ static int vfio_ap_mdev_create(struct kobject *kobj,
>> struct mdev_device *mdev)
>>  	list_add(&matrix_mdev->node, &matrix_dev->mdev_list);
>>  	mutex_unlock(&matrix_dev->lock);
>>
>> +	mdev_set_class_id(mdev, MDEV_ID_VFIO);
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/vfio/mdev/mdev_core.c
>> b/drivers/vfio/mdev/mdev_core.c
>> index b558d4cfd082..8764cf4a276d 100644
>> --- a/drivers/vfio/mdev/mdev_core.c
>> +++ b/drivers/vfio/mdev/mdev_core.c
>> @@ -45,6 +45,12 @@ void mdev_set_drvdata(struct mdev_device *mdev,
>> void *data)
>>  }
>>  EXPORT_SYMBOL(mdev_set_drvdata);
>>
>> +void mdev_set_class_id(struct mdev_device *mdev, u16 id)
>> +{
>> +	mdev->class_id = id;
>> +}
>> +EXPORT_SYMBOL(mdev_set_class_id);
>> +
>>  struct device *mdev_dev(struct mdev_device *mdev)
>>  {
>>  	return &mdev->dev;
>> @@ -135,6 +141,7 @@ static int mdev_device_remove_cb(struct device
>> *dev, void *data)
>>   * mdev_register_device : Register a device
>>   * @dev: device structure representing parent device.
>>   * @ops: Parent device operation structure to be registered.
>> + * @id: device id.
> class id.


Right, will fix.

Thanks


^ permalink raw reply

* Re: [PATCH] net: use unlikely for dql_avail case
From: Daniel Borkmann @ 2019-09-25 12:25 UTC (permalink / raw)
  To: xiaolinkui; +Cc: davem, netdev
In-Reply-To: <20190925024043.31030-1-xiaolinkui@kylinos.cn>

On Wed, Sep 25, 2019 at 10:40:43AM +0800, xiaolinkui wrote:
> This is an unlikely case, use unlikely() on it seems logical.
> 
> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>

It's already here [0], but should probably rather get reverted instead
due to lack of a more elaborate reasoning on why it needs to be done
this way instead of letting compiler do it's job in this case. "Seems
logical" is never a good technical explanation. Do you have any better
analysis you performed prior to submitting the patch (twice by now)?

Thanks,
Daniel

  [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3acd33d840d3ea3e1233d234605c85cbbf26054

> ---
>  include/linux/netdevice.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 88292953aa6f..005f3da1b13d 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3270,7 +3270,7 @@ static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
>  	 */
>  	smp_mb();
>  
> -	if (dql_avail(&dev_queue->dql) < 0)
> +	if (unlikely(dql_avail(&dev_queue->dql) < 0))
>  		return;
>  
>  	if (test_and_clear_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state))
> -- 
> 2.17.1
> 
> 
> 

^ permalink raw reply

* [PATCH net-next] net/sched: Set default of CONFIG_NET_TC_SKB_EXT to N
From: Paul Blakey @ 2019-09-25 12:28 UTC (permalink / raw)
  To: Pravin Shelar, Daniel Borkmann, Jakub Kicinski, Vlad Buslov,
	David Miller, netdev@vger.kernel.org, Jiri Pirko, Cong Wang,
	Jamal Hadi Salim, Simon Horman, Or Gerlitz
  Cc: Paul Blakey

This a new feature, it is preferred that it defaults to N.
We will probe the feature support from userspace before actually using it.

Fixes: 95a7233c452a ('net: openvswitch: Set OvS recirc_id from tc chain index')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
---
 net/sched/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index b3faafe..4bb10b7 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -966,7 +966,6 @@ config NET_IFE_SKBTCINDEX
 config NET_TC_SKB_EXT
 	bool "TC recirculation support"
 	depends on NET_CLS_ACT
-	default y if NET_CLS_ACT
 	select SKB_EXTENSIONS
 
 	help
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH V2 6/8] mdev: introduce virtio device and its device ops
From: Jason Wang @ 2019-09-25 12:45 UTC (permalink / raw)
  To: Tian, Kevin, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
	alex.williamson@redhat.com, mst@redhat.com, Bie, Tiwei
  Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
	cohuck@redhat.com, maxime.coquelin@redhat.com, Liang, Cunming,
	Wang, Zhihong, rob.miller@broadcom.com, Wang, Xiao W,
	haotian.wang@sifive.com, zhenyuw@linux.intel.com, Wang, Zhi A,
	jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
	Vivi, Rodrigo, airlied@linux.ie, daniel@ffwll.ch,
	farman@linux.ibm.com, pasic@linux.ibm.com, sebott@linux.ibm.com,
	oberpar@linux.ibm.com, heiko.carstens@de.ibm.com,
	gor@linux.ibm.com, borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
	freude@linux.ibm.com, Zhu, Lingshan, idos@mellanox.com,
	eperezma@redhat.com, lulu@redhat.com, parav@mellanox.com,
	christophe.de.dinechin@gmail.com
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D19D58F7DA@SHSMSX104.ccr.corp.intel.com>


On 2019/9/25 下午5:09, Tian, Kevin wrote:
>> From: Jason Wang [mailto:jasowang@redhat.com]
>> Sent: Tuesday, September 24, 2019 9:54 PM
>>
>> This patch implements basic support for mdev driver that supports
>> virtio transport for kernel virtio driver.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>   include/linux/mdev.h        |   2 +
>>   include/linux/virtio_mdev.h | 145
>> ++++++++++++++++++++++++++++++++++++
>>   2 files changed, 147 insertions(+)
>>   create mode 100644 include/linux/virtio_mdev.h
>>
>> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
>> index 3414307311f1..73ac27b3b868 100644
>> --- a/include/linux/mdev.h
>> +++ b/include/linux/mdev.h
>> @@ -126,6 +126,8 @@ struct mdev_device *mdev_from_dev(struct device
>> *dev);
>>
>>   enum {
>>   	MDEV_ID_VFIO = 1,
>> +	MDEV_ID_VIRTIO = 2,
>> +	MDEV_ID_VHOST = 3,
>>   	/* New entries must be added here */
>>   };
>>
>> diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h
>> new file mode 100644
>> index 000000000000..d1a40a739266
>> --- /dev/null
>> +++ b/include/linux/virtio_mdev.h
>> @@ -0,0 +1,145 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Virtio mediated device driver
>> + *
>> + * Copyright 2019, Red Hat Corp.
>> + *     Author: Jason Wang <jasowang@redhat.com>
>> + */
>> +#ifndef _LINUX_VIRTIO_MDEV_H
>> +#define _LINUX_VIRTIO_MDEV_H
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/mdev.h>
>> +#include <uapi/linux/vhost.h>
>> +
>> +#define VIRTIO_MDEV_DEVICE_API_STRING		"virtio-mdev"
>> +#define VIRTIO_MDEV_VERSION 0x1
> Just be curious. is this version identical to virtio spec version that below
> callbacks are created for, or just irrelevant?


It could be a hint but basically it's a way for userspace driver 
compatibility. For kernel we don't need this.


>
>> +
>> +struct virtio_mdev_callback {
>> +	irqreturn_t (*callback)(void *data);
>> +	void *private;
>> +};
>> +
>> +/**
>> + * struct vfio_mdev_device_ops - Structure to be registered for each
>> + * mdev device to register the device to virtio-mdev module.
>> + *
>> + * @set_vq_address:		Set the address of virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@desc_area: address of desc area
>> + *				@driver_area: address of driver area
>> + *				@device_area: address of device area
>> + *				Returns integer: success (0) or error (< 0)
>> + * @set_vq_num:		Set the size of virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@num: the size of virtqueue
>> + * @kick_vq:			Kick the virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + * @set_vq_cb:			Set the interrut calback function for
>> + *				a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@cb: virtio-mdev interrupt callback
>> structure
>> + * @set_vq_ready:		Set ready status for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@ready: ready (true) not ready(false)
>> + * @get_vq_ready:		Get ready status for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				Returns boolean: ready (true) or not (false)
>> + * @set_vq_state:		Set the state for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				@state: virtqueue state (last_avail_idx)
>> + *				Returns integer: success (0) or error (< 0)
>> + * @get_vq_state:		Get the state for a virtqueue
>> + *				@mdev: mediated device
>> + *				@idx: virtqueue index
>> + *				Returns virtqueue state (last_avail_idx)
>> + * @get_vq_align:		Get the virtqueue align requirement
>> + *				for the device
>> + *				@mdev: mediated device
>> + *				Returns virtqueue algin requirement
>> + * @get_features:		Get virtio features supported by the device
>> + *				@mdev: mediated device
>> + *				Returns the features support by the
>> + *				device
>> + * @get_features:		Set virtio features supported by the driver
>> + *				@mdev: mediated device
>> + *				@features: feature support by the driver
>> + *				Returns integer: success (0) or error (< 0)
>> + * @set_config_cb:		Set the config interrupt callback
>> + *				@mdev: mediated device
>> + *				@cb: virtio-mdev interrupt callback
>> structure
>> + * @get_device_id:		Get virtio device id
>> + *				@mdev: mediated device
>> + *				Returns u32: virtio device id
>> + * @get_vendor_id:		Get virtio vendor id
>> + *				@mdev: mediated device
>> + *				Returns u32: virtio vendor id
>> + * @get_status:		Get the device status
>> + *				@mdev: mediated device
>> + *				Returns u8: virtio device status
>> + * @set_status:		Set the device status
>> + *				@mdev: mediated device
>> + *				@status: virtio device status
>> + * @get_config:		Read from device specific confiugration
>> space
> configuration (and similar typos downward)


Let me fix.


>
>> + *				@mdev: mediated device
>> + *				@offset: offset from the beginning of
>> + *				configuration space
>> + *				@buf: buffer used to read to
>> + *				@len: the length to read from
>> + *				configration space
>> + * @set_config:		Write to device specific confiugration space
>> + *				@mdev: mediated device
>> + *				@offset: offset from the beginning of
>> + *				configuration space
>> + *				@buf: buffer used to write from
>> + *				@len: the length to write to
>> + *				configration space
>> + * @get_version:		Get the version of virtio mdev device
>> + *				@mdev: mediated device
>> + *				Returns integer: version of the device
>> + * @get_generation:		Get device generaton
>> + *				@mdev: mediated device
>> + *				Returns u32: device generation
>> + */
>> +struct virtio_mdev_device_ops {
>> +	/* Virtqueue ops */
>> +	int (*set_vq_address)(struct mdev_device *mdev,
>> +			      u16 idx, u64 desc_area, u64 driver_area,
>> +			      u64 device_area);
>> +	void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num);
>> +	void (*kick_vq)(struct mdev_device *mdev, u16 idx);
>> +	void (*set_vq_cb)(struct mdev_device *mdev, u16 idx,
>> +			  struct virtio_mdev_callback *cb);
>> +	void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool
>> ready);
>> +	bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx);
>> +	int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state);
>> +	u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx);
>> +
>> +	/* Device ops */
>> +	u16 (*get_vq_align)(struct mdev_device *mdev);
>> +	u64 (*get_features)(struct mdev_device *mdev);
>> +	int (*set_features)(struct mdev_device *mdev, u64 features);
>> +	void (*set_config_cb)(struct mdev_device *mdev,
>> +			      struct virtio_mdev_callback *cb);
>> +	u16 (*get_queue_max)(struct mdev_device *mdev);
>> +	u32 (*get_device_id)(struct mdev_device *mdev);
>> +	u32 (*get_vendor_id)(struct mdev_device *mdev);
>> +	u8 (*get_status)(struct mdev_device *mdev);
>> +	void (*set_status)(struct mdev_device *mdev, u8 status);
>> +	void (*get_config)(struct mdev_device *mdev, unsigned int offset,
>> +			   void *buf, unsigned int len);
>> +	void (*set_config)(struct mdev_device *mdev, unsigned int offset,
>> +			   const void *buf, unsigned int len);
>> +	int (*get_version)(struct mdev_device *mdev);
>> +	u32 (*get_generation)(struct mdev_device *mdev);
>> +};
> I'm not sure how stable above ops are.


It's the kernel internal API, so there's no strict requirement for this. 
We will export a version value for userspace for compatibility.


> Does it make sense if defining
> just two callbacks here, e.g. vq_ctrl and device_ctrl, and then let the
> vendor driver to handle specific ops in each category (similar to how
> ioctl works)?


My understanding is that it introduce another indirection, you still 
need to differ from different command, and it's less flexible than 
direct callback.

What's the value of doing this?

Thanks


>
> Thanks
> Kevin
>

^ permalink raw reply

* [PATCH] sunrpc: clean up indentation issue
From: Colin King @ 2019-09-25 13:09 UTC (permalink / raw)
  To: J . Bruce Fields, Chuck Lever, Trond Myklebust, Anna Schumaker,
	David S . Miller, linux-nfs, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

There are statements that are indented incorrectly, remove the
extraneous spacing.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/sunrpc/svc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 220b79988000..d11b70552c33 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1233,8 +1233,8 @@ svc_generic_init_request(struct svc_rqst *rqstp,
 
 	if (rqstp->rq_vers >= progp->pg_nvers )
 		goto err_bad_vers;
-	  versp = progp->pg_vers[rqstp->rq_vers];
-	  if (!versp)
+	versp = progp->pg_vers[rqstp->rq_vers];
+	if (!versp)
 		goto err_bad_vers;
 
 	/*
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/2] brcmfmac: fix suspend/resume when power is cut off
From: Adrian Ratiu @ 2019-09-25 13:44 UTC (permalink / raw)
  To: brcm80211-dev-list.pdl
  Cc: linux-wireless, netdev, linux-kernel, Gustavo Padovan
In-Reply-To: <20190925134458.1413790-1-adrian.ratiu@collabora.com>

brcmfmac assumed the wifi device always remains powered on and thus
hardcoded the MMC_PM_KEEP_POWER flag expecting the wifi device to
remain on even during suspend/resume cycles.

This is not always the case, some appliances cut power to everything
connected via SDIO for efficiency reasons and this leads to wifi not
being usable after coming out of suspend because the device was not
correctly reinitialized.

So we check for the keep_power capability and if it's not present then
we remove the device and probe it again during resume to mirror what's
happening in hardware and ensure correct reinitialization in the case
when MMC_PM_KEEP_POWER is not supported.

Suggested-by: Gustavo Padovan <gustavo.padovan@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 .../broadcom/brcm80211/brcmfmac/bcmsdh.c      | 53 ++++++++++++++-----
 1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index fc12598b2dd3..96fd8e2bf773 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -1108,7 +1108,8 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 	struct sdio_func *func;
 	struct brcmf_bus *bus_if;
 	struct brcmf_sdio_dev *sdiodev;
-	mmc_pm_flag_t sdio_flags;
+	mmc_pm_flag_t pm_caps, sdio_flags;
+	int ret = 0;
 
 	func = container_of(dev, struct sdio_func, dev);
 	brcmf_dbg(SDIO, "Enter: F%d\n", func->num);
@@ -1119,19 +1120,33 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 	bus_if = dev_get_drvdata(dev);
 	sdiodev = bus_if->bus_priv.sdio;
 
-	brcmf_sdiod_freezer_on(sdiodev);
-	brcmf_sdio_wd_timer(sdiodev->bus, 0);
+	pm_caps = sdio_get_host_pm_caps(func);
+
+	if (pm_caps & MMC_PM_KEEP_POWER) {
+		/* preserve card power during suspend */
+		brcmf_sdiod_freezer_on(sdiodev);
+		brcmf_sdio_wd_timer(sdiodev->bus, 0);
+
+		sdio_flags = MMC_PM_KEEP_POWER;
+		if (sdiodev->wowl_enabled) {
+			if (sdiodev->settings->bus.sdio.oob_irq_supported)
+				enable_irq_wake(sdiodev->settings->bus.sdio.oob_irq_nr);
+			else
+				sdio_flags |= MMC_PM_WAKE_SDIO_IRQ;
+		}
+
+		if (sdio_set_host_pm_flags(sdiodev->func1, sdio_flags))
+			brcmf_err("Failed to set pm_flags %x\n", sdio_flags);
 
-	sdio_flags = MMC_PM_KEEP_POWER;
-	if (sdiodev->wowl_enabled) {
-		if (sdiodev->settings->bus.sdio.oob_irq_supported)
-			enable_irq_wake(sdiodev->settings->bus.sdio.oob_irq_nr);
-		else
-			sdio_flags |= MMC_PM_WAKE_SDIO_IRQ;
+	} else {
+		/* power will be cut so remove device, probe again in resume */
+		brcmf_sdiod_intr_unregister(sdiodev);
+		ret = brcmf_sdiod_remove(sdiodev);
+		if (ret)
+			brcmf_err("Failed to remove device on suspend\n");
 	}
-	if (sdio_set_host_pm_flags(sdiodev->func1, sdio_flags))
-		brcmf_err("Failed to set pm_flags %x\n", sdio_flags);
-	return 0;
+
+	return ret;
 }
 
 static int brcmf_ops_sdio_resume(struct device *dev)
@@ -1139,13 +1154,23 @@ static int brcmf_ops_sdio_resume(struct device *dev)
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
 	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
 	struct sdio_func *func = container_of(dev, struct sdio_func, dev);
+	mmc_pm_flag_t pm_caps = sdio_get_host_pm_caps(func);
+	int ret = 0;
 
 	brcmf_dbg(SDIO, "Enter: F%d\n", func->num);
 	if (func->num != 2)
 		return 0;
 
-	brcmf_sdiod_freezer_off(sdiodev);
-	return 0;
+	if (!(pm_caps & MMC_PM_KEEP_POWER)) {
+		/* bus was powered off and device removed, probe again */
+		ret = brcmf_sdiod_probe(sdiodev);
+		if (ret)
+			brcmf_err("Failed to probe device on resume\n");
+	} else {
+		brcmf_sdiod_freezer_off(sdiodev);
+	}
+
+	return ret;
 }
 
 static const struct dev_pm_ops brcmf_sdio_pm_ops = {
-- 
2.23.0


^ permalink raw reply related

* [PATCH 1/2] brcmfmac: don't WARN when there are no requests
From: Adrian Ratiu @ 2019-09-25 13:44 UTC (permalink / raw)
  To: brcm80211-dev-list.pdl; +Cc: linux-wireless, netdev, linux-kernel, Martyn Welch

When n_reqs == 0 there is nothing to do so it doesn't make sense to
search for requests and issue a warning because none is found.

Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c
index 14e530601ef3..fabfbb0b40b0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c
@@ -57,6 +57,10 @@ static int brcmf_pno_remove_request(struct brcmf_pno_info *pi, u64 reqid)
 
 	mutex_lock(&pi->req_lock);
 
+	/* Nothing to do if we have no requests */
+	if (pi->n_reqs == 0)
+		goto done;
+
 	/* find request */
 	for (i = 0; i < pi->n_reqs; i++) {
 		if (pi->reqs[i]->reqid == reqid)
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH net v2 0/3] net/smc: move some definitions to UAPI
From: David Miller @ 2019-09-25 14:16 UTC (permalink / raw)
  To: ubraun; +Cc: esyr, netdev, linux-kernel, kgraul
In-Reply-To: <20af78a4-ded5-57ca-bd77-303cc7a59cf5@linux.ibm.com>

From: Ursula Braun <ubraun@linux.ibm.com>
Date: Wed, 25 Sep 2019 14:10:05 +0200

> we have to admit that it is already late for these patches. Nevertheless
> we think it is better to come up with them now than never. We doubt there
> exists already much userland code for it - except our own IBM-provided
> package smc-tools. Thus we appreciate acceptance of these patches.

Ursula, it's going to break the build of userland code.

I consider that unacceptable.

^ permalink raw reply

* Re: [PATCH net-next] net/sched: Set default of CONFIG_NET_TC_SKB_EXT to N
From: David Miller @ 2019-09-25 14:17 UTC (permalink / raw)
  To: paulb
  Cc: pshelar, daniel, jakub.kicinski, vladbu, netdev, jiri,
	xiyou.wangcong, jhs, simon.horman, gerlitz.or
In-Reply-To: <1569414509-22445-1-git-send-email-paulb@mellanox.com>

From: Paul Blakey <paulb@mellanox.com>
Date: Wed, 25 Sep 2019 15:28:29 +0300

> This a new feature, it is preferred that it defaults to N.
> We will probe the feature support from userspace before actually using it.
> 
> Fixes: 95a7233c452a ('net: openvswitch: Set OvS recirc_id from tc chain index')
> Signed-off-by: Paul Blakey <paulb@mellanox.com>

net-next is not an appropriate target tree

^ permalink raw reply

* Re: Fw: [Bug 204903] New: unable to create vrf interface when ipv6.disable=1
From: David Ahern @ 2019-09-25 14:22 UTC (permalink / raw)
  To: Michal Kubecek, netdev; +Cc: Stephen Hemminger
In-Reply-To: <20190925060109.GG22507@unicorn.suse.cz>

On 9/25/19 12:01 AM, Michal Kubecek wrote:
> Not sure if it's the only problem but vrf_fib_rule() checks
> ipv6_mod_enabled() for AF_INET6 but not for RTNL_FAMILY_IP6MR.

yes, that's the problem. sending a patch soon.

^ permalink raw reply

* [PATCH net] vrf: Do not attempt to create IPv6 mcast rule if IPv6 is disabled
From: David Ahern @ 2019-09-25 14:53 UTC (permalink / raw)
  To: davem, jakub.kicinski; +Cc: netdev, David Ahern, Patrick Ruddy

From: David Ahern <dsahern@gmail.com>

A user reported that vrf create fails when IPv6 is disabled at boot using
'ipv6.disable=1':
   https://bugzilla.kernel.org/show_bug.cgi?id=204903

The failure is adding fib rules at create time. Add RTNL_FAMILY_IP6MR to
the check in vrf_fib_rule if ipv6_mod_enabled is disabled.

Fixes: e4a38c0c4b27 ("ipv6: add vrf table handling code for ipv6 mcast")
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Patrick Ruddy <pruddy@vyatta.att-mail.com>
---
 drivers/net/vrf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 6e84328bdd40..a4b38a980c3c 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1154,7 +1154,8 @@ static int vrf_fib_rule(const struct net_device *dev, __u8 family, bool add_it)
 	struct sk_buff *skb;
 	int err;
 
-	if (family == AF_INET6 && !ipv6_mod_enabled())
+	if ((family == AF_INET6 || family == RTNL_FAMILY_IP6MR) &&
+	    !ipv6_mod_enabled())
 		return 0;
 
 	skb = nlmsg_new(vrf_fib_rule_nl_size(), GFP_KERNEL);
-- 
2.11.0


^ permalink raw reply related

* RE: [PATCH] net: stmmac: Fix ASSERT_RTNL() warning on suspend/resume
From: Loys Ollivier @ 2019-09-25 15:00 UTC (permalink / raw)
  To: Jose Abreu, Russell King, Maxime Coquelin
  Cc: David S. Miller, Giuseppe Cavallaro, Alexandre Torgue,
	netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <BN8PR12MB3266A4C33D234165A0F2A978D38F0@BN8PR12MB3266.namprd12.prod.outlook.com>

On Tue 17 Sep 2019 at 10:12, Jose Abreu <Jose.Abreu@synopsys.com> wrote:

> From: Loys Ollivier <lollivier@baylibre.com>
> Date: Sep/17/2019, 11:02:36 (UTC+00:00)
>
>> rtnl_lock needs to be taken before calling phylink_start/stop to lock the
>> network stack.
>> Fix ASSERT_RTNL() warnings by protecting such calls with lock/unlock.
>> 
>> Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
>> Signed-off-by: Loys Ollivier <lollivier@baylibre.com>
>
> I already sent a fix for this. Please see in -net:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/driv
> ers/net/ethernet/stmicro/stmmac?id=19e13cb27b998ff49f07e399b5871bfe5ba7e3
> f0

Ah good catch :)

>
> ---
> Thanks,
> Jose Miguel Abreu

^ permalink raw reply

* [PATCH net v2] net/sched: Set default of CONFIG_NET_TC_SKB_EXT to N
From: Paul Blakey @ 2019-09-25 15:02 UTC (permalink / raw)
  To: Pravin Shelar, Daniel Borkmann, Jakub Kicinski, Vlad Buslov,
	David Miller, netdev@vger.kernel.org, Jiri Pirko, Cong Wang,
	Jamal Hadi Salim, Simon Horman, Or Gerlitz
  Cc: Paul Blakey

This a new feature, it is preferred that it defaults to N.
We will probe the feature support from userspace before actually using it.

Fixes: 95a7233c452a ('net: openvswitch: Set OvS recirc_id from tc chain index')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
---
Changelog:
	v1->v2: Changed target to net.

 net/sched/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index b3faafe..4bb10b7 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -966,7 +966,6 @@ config NET_IFE_SKBTCINDEX
 config NET_TC_SKB_EXT
 	bool "TC recirculation support"
 	depends on NET_CLS_ACT
-	default y if NET_CLS_ACT
 	select SKB_EXTENSIONS
 
 	help
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH] sunrpc: clean up indentation issue
From: J . Bruce Fields @ 2019-09-25 15:13 UTC (permalink / raw)
  To: Colin King
  Cc: Chuck Lever, Trond Myklebust, Anna Schumaker, David S . Miller,
	linux-nfs, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190925130930.13076-1-colin.king@canonical.com>

Applied, thanks.--b.

On Wed, Sep 25, 2019 at 02:09:30PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> There are statements that are indented incorrectly, remove the
> extraneous spacing.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  net/sunrpc/svc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 220b79988000..d11b70552c33 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -1233,8 +1233,8 @@ svc_generic_init_request(struct svc_rqst *rqstp,
>  
>  	if (rqstp->rq_vers >= progp->pg_nvers )
>  		goto err_bad_vers;
> -	  versp = progp->pg_vers[rqstp->rq_vers];
> -	  if (!versp)
> +	versp = progp->pg_vers[rqstp->rq_vers];
> +	if (!versp)
>  		goto err_bad_vers;
>  
>  	/*
> -- 
> 2.20.1

^ permalink raw reply

* IPv6 issue
From: Levente @ 2019-09-25 15:25 UTC (permalink / raw)
  To: netdev

Dear all,


I don't know if this is the right place to ask, so please be gentle.

I have a router running OpenWRT, and it fails the some test cases
defined by the IPv6 forum.

https://www.ipv6ready.org/docs/Core_Conformance_Latest.pdf

It fails 1.2.3 / 1.2.4 / 1.2.5 / 1.2.8 test cases.

My question is if there's any particular settings (either compile or
running time) in the kernel that affects the outcome of these test
cases?

Other question is that is there any open source test program to test
the kernel's IPv6 stack against this test specifications?


Thanks,
Levente

^ permalink raw reply

* [PATCH] i40e: prevent memory leak in i40e_setup_macvlans
From: Navid Emamdoost @ 2019-09-25 15:48 UTC (permalink / raw)
  Cc: emamd001, kjlu, smccaman, Navid Emamdoost, Jeff Kirsher,
	David S. Miller, intel-wired-lan, netdev, linux-kernel

In i40e_setup_macvlans if i40e_setup_channel fails the allocated memory
for ch should be released.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 6031223eafab..7d4a14c3fe58 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -7168,6 +7168,7 @@ static int i40e_setup_macvlans(struct i40e_vsi *vsi, u16 macvlan_cnt, u16 qcnt,
 		ch->num_queue_pairs = qcnt;
 		if (!i40e_setup_channel(pf, vsi, ch)) {
 			ret = -EINVAL;
+			kfree(ch);
 			goto err_free;
 		}
 		ch->parent_vsi = vsi;
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH bpf-next v11 4/4] tools/testing/selftests/bpf: Add self-tests for new helper.
From: Yonghong Song @ 2019-09-25 16:07 UTC (permalink / raw)
  To: Carlos Neira, netdev@vger.kernel.org
  Cc: ebiederm@xmission.com, brouer@redhat.com, bpf@vger.kernel.org
In-Reply-To: <20190924152005.4659-5-cneirabustos@gmail.com>



On 9/24/19 8:20 AM, Carlos Neira wrote:
> Self tests added for new helper
> 
> Signed-off-by: Carlos Neira <cneirabustos@gmail.com>
> ---
>   tools/testing/selftests/bpf/Makefile          |   2 +-
>   tools/testing/selftests/bpf/bpf_helpers.h     |   3 +
>   .../selftests/bpf/progs/test_pidns_kern.c     |  71 ++++++++
>   tools/testing/selftests/bpf/test_pidns.c      | 152 ++++++++++++++++++
>   4 files changed, 227 insertions(+), 1 deletion(-)
>   create mode 100644 tools/testing/selftests/bpf/progs/test_pidns_kern.c
>   create mode 100644 tools/testing/selftests/bpf/test_pidns.c
> 
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 7f3196af1ae4..d86b28aa8f44 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -28,7 +28,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
>   	test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
>   	test_cgroup_storage test_select_reuseport test_section_names \
>   	test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
> -	test_btf_dump test_cgroup_attach xdping
> +	test_btf_dump test_cgroup_attach xdping test_pidns

Could you fold test_pidns into test_progs?

>   
>   BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
>   TEST_GEN_FILES = $(BPF_OBJ_FILES)
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 6c4930bc6e2e..03d0e15ae29f 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -231,6 +231,9 @@ static int (*bpf_send_signal)(unsigned sig) = (void *)BPF_FUNC_send_signal;
>   static long long (*bpf_tcp_gen_syncookie)(struct bpf_sock *sk, void *ip,
>   					  int ip_len, void *tcp, int tcp_len) =
>   	(void *) BPF_FUNC_tcp_gen_syncookie;
> +static int (*bpf_get_ns_current_pid_tgid)(__u32 dev, __u64 inum) =
> +	(void *) BPF_FUNC_get_ns_current_pid_tgid;
> +
>   
>   /* llvm builtin functions that eBPF C program may use to
>    * emit BPF_LD_ABS and BPF_LD_IND instructions
> diff --git a/tools/testing/selftests/bpf/progs/test_pidns_kern.c b/tools/testing/selftests/bpf/progs/test_pidns_kern.c
> new file mode 100644
> index 000000000000..96cb707db3ee
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_pidns_kern.c
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018 Carlos Neira cneirabustos@gmail.com

2019

> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.

You do not need the above statement, which is covered by
"SPDX-License-Identifier: GPL-2.0".

> + */
> +
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u64);
> +} ns_inum_map SEC(".maps");
> +
> +struct  {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u32);
> +} ns_dev_map SEC(".maps");
> +
> +struct   {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u32);
> +} pidmap SEC(".maps");

Can we make the value __u64 to include
both pid and tid to compare both?

> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u64);
> +} ns_pid_map SEC(".maps");
> +
> 

The above four one-element maps are perfectly examples
to use static global variables which are supported
by the kernel.

You can take a look at the patch
   https://patchwork.ozlabs.org/patch/1081014/
which shows how user space can modify the map
values. In the future, we could have a better
interface to read/update those static variable
values.

> +
> +SEC("tracepoint/syscalls/sys_enter_nanosleep")
> +int trace(void *ctx)
> +{
> +	__u32 key = 0, *expected_pid, *dev;

expected_pid => __u64 *?

> +	__u64 *val, *inum, nspid;
> +	__u32 pid;
> +
> +	dev = bpf_map_lookup_elem(&ns_dev_map, &key);
> +	if (!dev)
> +		return 0;
> +
> +	inum = bpf_map_lookup_elem(&ns_inum_map, &key);
> +	if (!inum)
> +		return 0;
> +
> +	nspid = bpf_get_ns_current_pid_tgid(*dev, *inum);
> +	expected_pid = bpf_map_lookup_elem(&pidmap, &key);
> +
> +	if (!expected_pid || *expected_pid != nspid)
> +		return 0;
> +
> +	val = bpf_map_lookup_elem(&ns_pid_map, &key);
> +	if (val)
> +		*val = nspid;
> +
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
> diff --git a/tools/testing/selftests/bpf/test_pidns.c b/tools/testing/selftests/bpf/test_pidns.c
> new file mode 100644
> index 000000000000..088f8025f2bf
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_pidns.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018 Carlos Neira cneirabustos@gmail.com

2019

> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.

ditto. you do not need the above statement.

> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <syscall.h>
> +#include <unistd.h>
> +#include <linux/perf_event.h>
> +#include <sys/ioctl.h>
> +#include <sys/time.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf.h>
> +#include <bpf/libbpf.h>
> +
> +#include "cgroup_helpers.h"
> +#include "bpf_rlimit.h"
> +
> +#define CHECK(condition, tag, format...) ({		\
> +	int __ret = !!(condition);			\
> +	if (__ret) {					\
> +		printf("%s:FAIL:%s ", __func__, tag);	\
> +		printf(format);				\
> +	} else {					\
> +		printf("%s:PASS:%s\n", __func__, tag);	\
> +	}						\
> +	__ret;						\
> +})
> +
> +static int bpf_find_map(const char *test, struct bpf_object *obj,
> +			const char *name)
> +{
> +	struct bpf_map *map;
> +
> +	map = bpf_object__find_map_by_name(obj, name);
> +	if (!map)
> +		return -1;
> +	return bpf_map__fd(map);

The 'test' argument is not used here.
Also, we have bpf_object__find_map_fd_by_name() API, you can
use it and you do not need this function.

> +}
> +
> +
> +int main(int argc, char **argv)
> +{
> +	int pidmap_fd, ns_inum_map_fd, ns_dev_map_fd, ns_pid_map_fd;
> +	const char *probe_name = "syscalls/sys_enter_nanosleep";
> +	const char *file = "test_pidns_kern.o";
> +	int err, bytes, efd, prog_fd, pmu_fd;
> +	struct perf_event_attr attr = {};
> +	struct bpf_object *obj;
> +	__u32 nspid = 0;
> +	__u32 key = 0, pid;
> +	int exit_code = 1;

to make it reverse Christmas tree style?

> +	struct stat st;
> +	char buf[256];
> +
> +	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
> +	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
> +		goto cleanup_cgroup_env;
> +
> +	ns_dev_map_fd = bpf_find_map(__func__, obj, "ns_dev_map");
> +	if (CHECK(ns_dev_map_fd < 0, "bpf_find_map", "err %d errno %d\n",
> +		  ns_dev_map_fd, errno))
> +		goto close_prog;
> +
> +	ns_inum_map_fd = bpf_find_map(__func__, obj, "ns_inum_map");
> +	if (CHECK(ns_inum_map_fd < 0, "bpf_find_map", "err %d errno %d\n",
> +		  ns_inum_map_fd, errno))
> +		goto close_prog;
> +
> +	ns_pid_map_fd = bpf_find_map(__func__, obj, "ns_pid_map");
> +	if (CHECK(ns_pid_map_fd < 0, "bpf_find_map", "err %d errno %d\n",
> +		  ns_pid_map_fd, errno))
> +		goto close_prog;
> +
> +	pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
> +	if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
> +		  pidmap_fd, errno))
> +		goto close_prog;
> +
> +	pid = getpid();
> +	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);

maybe
	id = (__u64) getpid() << 32 | gettid();
	bpf_map_update_elem(pidmap_fd, &key, &id, 0);

In your original above kernel code, you actually compare user space pid
to kernel tid, which is okay for this program as it is the main thread 
and tid == pid, but the mechanism is wrong.

> +
> +	if (stat("/proc/self/ns/pid", &st))
> +		goto close_prog;
> +
> +	bpf_map_update_elem(ns_inum_map_fd, &key, &st.st_ino, 0);
> +	bpf_map_update_elem(ns_dev_map_fd, &key, &st.st_dev, 0);
> +
> +
> +	snprintf(buf, sizeof(buf),
> +		 "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> +	efd = open(buf, O_RDONLY, 0);
> +	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> +		goto close_prog;
> +	bytes = read(efd, buf, sizeof(buf));
> +	close(efd);
> +	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
> +		  "bytes %d errno %d\n", bytes, errno))
> +		goto close_prog;
> +
> +	attr.config = strtol(buf, NULL, 0);
> +	attr.type = PERF_TYPE_TRACEPOINT;
> +	attr.sample_type = PERF_SAMPLE_RAW;
> +	attr.sample_period = 1;
> +	attr.wakeup_events = 1;
> +
> +	pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
> +	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
> +		  errno))
> +		goto close_prog;
> +
> +	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> +	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
> +		  errno))
> +		goto close_pmu;
> +
> +	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> +	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
> +		  errno))
> +		goto close_pmu;

The whole thing above related to tracepoint can be simplified. Please 
see prog_tests/stacktrace_map.c.

> +
> +	/* trigger some syscalls */
> +	sleep(1);
> +
> +	err = bpf_map_lookup_elem(ns_pid_map_fd, &key, &nspid);
> +	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
> +		goto close_pmu;
> +
> +	if (CHECK(nspid != pid, "compare nspid vs pid",
> +		  "kern nspid %u user pid %u", nspid, pid))
> +		goto close_pmu;
> +
> +	exit_code = 0;
> +	printf("%s:PASS\n", argv[0]);
> +
> +close_pmu:
> +	close(pmu_fd);
> +close_prog:
> +	bpf_object__close(obj);
> +cleanup_cgroup_env:
> +	return exit_code;
> +}
> 

^ permalink raw reply

* Re: IPv6 issue
From: 吉藤英明 @ 2019-09-25 16:21 UTC (permalink / raw)
  To: Levente; +Cc: netdev, 吉藤英明, yoshfuji
In-Reply-To: <CACwWb3BE7msW6=XADuG2Di4xYnoJq5qScc4Wsu4xOS=ycYPDww@mail.gmail.com>

Hi,


2019年9月26日(木) 0:25 Levente <leventelist@gmail.com>:
>
> Dear all,
>
>
> I don't know if this is the right place to ask, so please be gentle.
>
> I have a router running OpenWRT, and it fails the some test cases
> defined by the IPv6 forum.
>
> https://www.ipv6ready.org/docs/Core_Conformance_Latest.pdf
>
> It fails 1.2.3 / 1.2.4 / 1.2.5 / 1.2.8 test cases.
>
> My question is if there's any particular settings (either compile or
> running time) in the kernel that affects the outcome of these test
> cases?

You should look into the result in detail; how it fails.

>
> Other question is that is there any open source test program to test
> the kernel's IPv6 stack against this test specifications?
>

You could find some "Self Test" suites:
https://www.ipv6ready.org/?page=documents&tag=ipv6-core-protocols

--yoshfuji

>
> Thanks,
> Levente

^ permalink raw reply

* KASAN: use-after-free Read in move_expired_inodes
From: syzbot @ 2019-09-25 16:56 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, netdev, syzkaller-bugs, viro

Hello,

syzbot found the following crash on:

HEAD commit:    b41dae06 Merge tag 'xfs-5.4-merge-7' of git://git.kernel.o..
git tree:       net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=135e0ee3600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=dfcf592db22b9132
dashboard link: https://syzkaller.appspot.com/bug?extid=8c1cf49d27ac637b47c0
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

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

==================================================================
BUG: KASAN: use-after-free in move_expired_inodes+0x8c8/0x9c0  
fs/fs-writeback.c:1241
Read of size 8 at addr ffff88805929a868 by task kworker/u4:8/10864

CPU: 1 PID: 10864 Comm: kworker/u4:8 Not tainted 5.3.0+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: writeback wb_workfn (flush-8:0)
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  print_address_description.cold+0xd4/0x306 mm/kasan/report.c:351
  __kasan_report.cold+0x1b/0x36 mm/kasan/report.c:482
  kasan_report+0x12/0x17 mm/kasan/common.c:618
  __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
  move_expired_inodes+0x8c8/0x9c0 fs/fs-writeback.c:1241
  queue_io+0x1c5/0x590 fs/fs-writeback.c:1290
  wb_writeback+0xa99/0xd90 fs/fs-writeback.c:1882
  wb_check_start_all fs/fs-writeback.c:2010 [inline]
  wb_do_writeback fs/fs-writeback.c:2036 [inline]
  wb_workfn+0xb0e/0x11e0 fs/fs-writeback.c:2070
  process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
  worker_thread+0x98/0xe40 kernel/workqueue.c:2415
  kthread+0x361/0x430 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Allocated by task 21365:
  save_stack+0x23/0x90 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_kmalloc mm/kasan/common.c:493 [inline]
  __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:466
  kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:501
  slab_post_alloc_hook mm/slab.h:520 [inline]
  slab_alloc mm/slab.c:3319 [inline]
  kmem_cache_alloc+0x121/0x710 mm/slab.c:3483
  ext4_alloc_inode+0x1f/0x640 fs/ext4/super.c:1073
  alloc_inode+0x68/0x1e0 fs/inode.c:227
  new_inode_pseudo+0x19/0xf0 fs/inode.c:916
  new_inode+0x1f/0x40 fs/inode.c:945
  __ext4_new_inode+0x3d5/0x4e50 fs/ext4/ialloc.c:829
  ext4_create+0x236/0x5e0 fs/ext4/namei.c:2587
  lookup_open+0x12be/0x1a50 fs/namei.c:3224
  do_last fs/namei.c:3314 [inline]
  path_openat+0x14ac/0x4630 fs/namei.c:3525
  do_filp_open+0x1a1/0x280 fs/namei.c:3555
  do_sys_open+0x3fe/0x5d0 fs/open.c:1089
  __do_sys_openat fs/open.c:1116 [inline]
  __se_sys_openat fs/open.c:1110 [inline]
  __x64_sys_openat+0x9d/0x100 fs/open.c:1110
  do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 21405:
  save_stack+0x23/0x90 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_slab_free+0x102/0x150 mm/kasan/common.c:455
  kasan_slab_free+0xe/0x10 mm/kasan/common.c:463
  __cache_free mm/slab.c:3425 [inline]
  kmem_cache_free+0x86/0x320 mm/slab.c:3693
  ext4_free_in_core_inode+0x28/0x30 fs/ext4/super.c:1120
  i_callback+0x44/0x80 fs/inode.c:216
  __rcu_reclaim kernel/rcu/rcu.h:222 [inline]
  rcu_do_batch kernel/rcu/tree.c:2157 [inline]
  rcu_core+0x581/0x1560 kernel/rcu/tree.c:2377
  rcu_core_si+0x9/0x10 kernel/rcu/tree.c:2386
  __do_softirq+0x262/0x98c kernel/softirq.c:292

The buggy address belongs to the object at ffff88805929a500
  which belongs to the cache ext4_inode_cache(33:syz2) of size 2000
The buggy address is located 872 bytes inside of
  2000-byte region [ffff88805929a500, ffff88805929acd0)
The buggy address belongs to the page:
page:ffffea000164a680 refcount:1 mapcount:0 mapping:ffff88809b51f8c0  
index:0xffff88805929afff
flags: 0x1fffc0000000200(slab)
raw: 01fffc0000000200 ffffea00016446c8 ffffea00028cc3c8 ffff88809b51f8c0
raw: ffff88805929afff ffff88805929a500 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff88805929a700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff88805929a780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff88805929a800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                           ^
  ffff88805929a880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff88805929a900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


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

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

^ permalink raw reply

* Re: CONFIG_NET_TC_SKB_EXT
From: Edward Cree @ 2019-09-25 17:01 UTC (permalink / raw)
  To: Paul Blakey, Jakub Kicinski
  Cc: Pravin Shelar, Daniel Borkmann, Vlad Buslov, David Miller,
	netdev@vger.kernel.org, Jiri Pirko, Cong Wang, Jamal Hadi Salim,
	Simon Horman, Or Gerlitz
In-Reply-To: <4f99e2b6-0f09-9d2c-6300-dfc884d501a8@mellanox.com>

On 24/09/2019 12:48, Paul Blakey wrote:
> The 'miss' for all or nothing is easy, but the hard part is combining 
> all the paths a packet can take in software to a single 'all or nothing' 
> rule in hardware.
But you don't combine them to a single rule in hardware, because you
 have multiple sequential tables.  (I just spent the last few weeks
 telling our hardware guys that no, they can't just give us one big
 table and expect the driver to do all that combining, because as you
 say, it's 'the hard part'.)

> What if you 'miss' on the match for the tuple? You already did some 
> processing in hardware, so either you revert those, or you continue in 
> software where you left off  (the action ct).
But the only processing you did was to match stuff and generate metadata
 in the form of lookup keys (e.g. a ct_zone) for the next round of
 matching.  There's nothing to "revert" unless you've actually modified
 the packet before sending it to CT, and as I said I don't believe that's
 worth supporting.

> The all or nothing approach will require changing the software model to 
> allow
>
> merging the ct zone table matches into the hardware rules
I don't know how much more clearly I can say this: all-or-nothing does not
 require merging.  It just requires any actions that come before a matching
 stage (and that the hw doesn't have the capability to revert) to put a
 rule straight in the 'nothing' bucket.
So if you write
  chain 0 dst_mac aa:bb:cc:dd:ee:ff ct_state -trk  action vlan push blah action ct action goto chain X
 the driver can say -EOPNOTSUPP because you pushed a VLAN and might still
 miss in chain X.  But if you write
  chain 0 dst_mac aa:bb:cc:dd:ee:ff ct_state -trk  action ct action goto chain X
 then the driver will happily offload that because if you miss in the later
 lookups you've not altered the packet — the chain0-rule is *idempotent* so
 it doesn't matter if HW and SW both perform it.  (Or even all three of HW,
 tc and OvS.)

^ permalink raw reply

* RE: [PATCH] IB/mlx5: add checking for "vf" from do_setvfinfo()
From: Parav Pandit @ 2019-09-25 17:14 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: netdev@vger.kernel.org, Leon Romanovsky, Eli Cohen, Doug Ledford,
	Jason Gunthorpe, linux-rdma@vger.kernel.org,
	kernel-janitors@vger.kernel.org
In-Reply-To: <20190924091823.GM20699@kadam>

Hi Dan,

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Tuesday, September 24, 2019 4:21 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: netdev@vger.kernel.org; Leon Romanovsky <leon@kernel.org>; Eli Cohen
> <eli@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe
> <jgg@ziepe.ca>; linux-rdma@vger.kernel.org; kernel-janitors@vger.kernel.org
> Subject: Re: [PATCH] IB/mlx5: add checking for "vf" from do_setvfinfo()
> 
> On Thu, Apr 25, 2019 at 06:15:13AM +0000, Parav Pandit wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dan Carpenter <dan.carpenter@oracle.com>
> > > Sent: Wednesday, April 24, 2019 9:08 AM
> > > To: Parav Pandit <parav@mellanox.com>; netdev@vger.kernel.org
> > > Cc: Leon Romanovsky <leon@kernel.org>; Eli Cohen <eli@mellanox.com>;
> > > Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>;
> > > linux-rdma@vger.kernel.org; kernel-janitors@vger.kernel.org
> > > Subject: Re: [PATCH] IB/mlx5: add checking for "vf" from
> > > do_setvfinfo()
> > >
> > > I think I'm just going to ask netdev for an opinion on this.  It
> > > could be that we're just reading the code wrong...
> > >
> > > I'm getting a lot of Smatch warning about buffer underflows.  The
> > > problem is that Smatch marks everything from nla_data() as unknown
> > > and untrusted user data.  In do_setvfinfo() we get the "->vf" values
> > > from nla_data().  It starts as u32, but all the function pointers in
> > > net_device_ops use it as a signed integer.  Most of the functions
> > > return -EINVAL if "vf" is negative but there are at least 48 which
> > > potentially use negative values as an offset into an array.
> > >
> > > To me making "vf" a u32 throughout seems like a good idea but it's
> > > an extensive patch and I'm not really able to test it at all.
> >
> > I will be try to get you patch early next week for core and in mlx5,
> > tested on mlx5 VFs, that possibly you can carry forward?
> 
> Whatever happened with this?
> 
I had internal few patches that Leon and Saeed reviewed, but it needs more rework at core and driver level.
I haven't had chance to finish it.

> regards,
> dan carpenter

^ permalink raw reply


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