Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Andy Shevchenko @ 2019-09-11  9:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, Mika Westerberg, linux-kernel, linux-gpio,
	Andrew Lunn, David S. Miller, Florian Fainelli, Heiner Kallweit,
	Russell King, netdev
In-Reply-To: <20190911075215.78047-5-dmitry.torokhov@gmail.com>

On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> works with arbitrary firmware node.

I'm wondering if it's possible to step forward and replace
fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
in other cases in this series.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* [vhost:linux-next 14/17] include/linux/mmzone.h:815:3: error: implicit declaration of function 'move_page_to_reported_list'; did you mean 'move_to_free_list'?
From: kbuild test robot @ 2019-09-11  9:33 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: kbuild-all, kvm, virtualization, netdev, Michael S. Tsirkin

[-- Attachment #1: Type: text/plain, Size: 6194 bytes --]

tree:   https://kernel.googlesource.com/pub/scm/linux/kernel/git/mst/vhost.git linux-next
head:   39c226b6b576b23d6d558331e6895f02b0892555
commit: 50ed0c2ecb2e254a50e4ad775840f29d42cf6c00 [14/17] mm: Introduce Reported pages
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 50ed0c2ecb2e254a50e4ad775840f29d42cf6c00
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/page_reporting.h:5:0,
                    from <command-line>:0:
   include/linux/mmzone.h: In function 'add_to_free_list_tail':
   include/linux/mmzone.h:791:27: error: implicit declaration of function 'get_unreported_tail' [-Werror=implicit-function-declaration]
     struct list_head *tail = get_unreported_tail(zone, order, migratetype);
                              ^~~~~~~~~~~~~~~~~~~
   include/linux/mmzone.h:791:27: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
   include/linux/mmzone.h: In function 'move_to_free_list':
   include/linux/mmzone.h:807:27: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
     struct list_head *tail = get_unreported_tail(zone, order, migratetype);
                              ^~~~~~~~~~~~~~~~~~~
>> include/linux/mmzone.h:815:3: error: implicit declaration of function 'move_page_to_reported_list'; did you mean 'move_to_free_list'? [-Werror=implicit-function-declaration]
      move_page_to_reported_list(page, zone, migratetype);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
      move_to_free_list
   include/linux/mmzone.h: In function 'del_page_from_free_list':
   include/linux/mmzone.h:831:3: error: implicit declaration of function 'del_page_from_reported_list'; did you mean 'del_page_from_free_list'? [-Werror=implicit-function-declaration]
      del_page_from_reported_list(page, zone);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
      del_page_from_free_list
   In file included from <command-line>:0:0:
   include/linux/page_reporting.h: At top level:
   include/linux/page_reporting.h:74:1: error: conflicting types for 'get_unreported_tail'
    get_unreported_tail(struct zone *zone, unsigned int order, int migratetype)
    ^~~~~~~~~~~~~~~~~~~
   In file included from include/linux/page_reporting.h:5:0,
                    from <command-line>:0:
   include/linux/mmzone.h:791:27: note: previous implicit declaration of 'get_unreported_tail' was here
     struct list_head *tail = get_unreported_tail(zone, order, migratetype);
                              ^~~~~~~~~~~~~~~~~~~
   In file included from <command-line>:0:0:
   include/linux/page_reporting.h:106:20: warning: conflicting types for 'del_page_from_reported_list'
    static inline void del_page_from_reported_list(struct page *page,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/page_reporting.h:106:20: error: static declaration of 'del_page_from_reported_list' follows non-static declaration
   In file included from include/linux/page_reporting.h:5:0,
                    from <command-line>:0:
   include/linux/mmzone.h:831:3: note: previous implicit declaration of 'del_page_from_reported_list' was here
      del_page_from_reported_list(page, zone);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from <command-line>:0:0:
   include/linux/page_reporting.h:118:20: warning: conflicting types for 'move_page_to_reported_list'
    static inline void move_page_to_reported_list(struct page *page,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/page_reporting.h:118:20: error: static declaration of 'move_page_to_reported_list' follows non-static declaration
   In file included from include/linux/page_reporting.h:5:0,
                    from <command-line>:0:
   include/linux/mmzone.h:815:3: note: previous implicit declaration of 'move_page_to_reported_list' was here
      move_page_to_reported_list(page, zone, migratetype);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +815 include/linux/mmzone.h

   786	
   787	/* Used for pages not on another list */
   788	static inline void add_to_free_list_tail(struct page *page, struct zone *zone,
   789						 unsigned int order, int migratetype)
   790	{
 > 791		struct list_head *tail = get_unreported_tail(zone, order, migratetype);
   792	
   793		/*
   794		 * To prevent the unreported pages from being interleaved with the
   795		 * reported ones while we are actively processing pages we will use
   796		 * the head of the reported pages to determine the tail of the free
   797		 * list.
   798		 */
   799		list_add_tail(&page->lru, tail);
   800		zone->free_area[order].nr_free++;
   801	}
   802	
   803	/* Used for pages which are on another list */
   804	static inline void move_to_free_list(struct page *page, struct zone *zone,
   805					     unsigned int order, int migratetype)
   806	{
   807		struct list_head *tail = get_unreported_tail(zone, order, migratetype);
   808	
   809		/*
   810		 * We must get the tail for our target list before moving the page on
   811		 * the reported list as we will possibly be replacing the tail page of
   812		 * the list with our current page if it is reported.
   813		 */
   814		if (unlikely(PageReported(page)))
 > 815			move_page_to_reported_list(page, zone, migratetype);
   816	
   817		/*
   818		 * To prevent unreported pages from being mixed with the reported
   819		 * ones we add pages to the tail of the list. By doing this the function
   820		 * above can either label them as included in the reported list or not
   821		 * and the result will be consistent.
   822		 */
   823		list_move_tail(&page->lru, tail);
   824	}
   825	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50929 bytes --]

^ permalink raw reply

* Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
From: Michael S. Tsirkin @ 2019-09-11  9:36 UTC (permalink / raw)
  To: Jason Wang
  Cc: kvm, virtualization, netdev, linux-kernel, kwankhede,
	alex.williamson, cohuck, tiwei.bie, maxime.coquelin,
	cunming.liang, zhihong.wang, rob.miller, idos, xiao.w.wang,
	haotian.wang
In-Reply-To: <390647ae-0a53-5f2b-ccb0-28ed657636e6@redhat.com>

On Wed, Sep 11, 2019 at 10:38:39AM +0800, Jason Wang wrote:
> 
> On 2019/9/10 下午9:52, Michael S. Tsirkin wrote:
> > On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote:
> > > On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
> > > > > +#ifndef _LINUX_VIRTIO_MDEV_H
> > > > > +#define _LINUX_VIRTIO_MDEV_H
> > > > > +
> > > > > +#include <linux/interrupt.h>
> > > > > +#include <linux/vringh.h>
> > > > > +#include <uapi/linux/virtio_net.h>
> > > > > +
> > > > > +/*
> > > > > + * Ioctls
> > > > > + */
> > > > Pls add a bit more content here. It's redundant to state these
> > > > are ioctls. Much better to document what does each one do.
> > > 
> > > Ok.
> > > 
> > > 
> > > > > +
> > > > > +struct virtio_mdev_callback {
> > > > > +	irqreturn_t (*callback)(void *);
> > > > > +	void *private;
> > > > > +};
> > > > > +
> > > > > +#define VIRTIO_MDEV 0xAF
> > > > > +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
> > > > > +					 struct virtio_mdev_callback)
> > > > > +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
> > > > > +					struct virtio_mdev_callback)
> > > > Function pointer in an ioctl parameter? How does this ever make sense?
> > > 
> > > I admit this is hacky (casting).
> > > 
> > > 
> > > > And can't we use a couple of registers for this, and avoid ioctls?
> > > 
> > > Yes, how about something like interrupt numbers for each virtqueue and
> > > config?
> > Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then?
> 
> 
> You mean something like VIRTIO_PCI_COMMON_Q_MSIX? Then it becomes a PCI
> transport in fact. And using either MSIX or irq number is actually another
> layer of indirection. So I think we can just write callback function and
> parameter through registers.

I just realized, all these registers are just encoded so you
can pass stuff through read/write. But it can instead be
just a normal C function call with no messy encoding.
So why do we want to do this encoding?


> 
> > 
> > 
> > > > > +
> > > > > +#define VIRTIO_MDEV_DEVICE_API_STRING		"virtio-mdev"
> > > > > +
> > > > > +/*
> > > > > + * Control registers
> > > > > + */
> > > > > +
> > > > > +/* Magic value ("virt" string) - Read Only */
> > > > > +#define VIRTIO_MDEV_MAGIC_VALUE		0x000
> > > > > +
> > > > > +/* Virtio device version - Read Only */
> > > > > +#define VIRTIO_MDEV_VERSION		0x004
> > > > > +
> > > > > +/* Virtio device ID - Read Only */
> > > > > +#define VIRTIO_MDEV_DEVICE_ID		0x008
> > > > > +
> > > > > +/* Virtio vendor ID - Read Only */
> > > > > +#define VIRTIO_MDEV_VENDOR_ID		0x00c
> > > > > +
> > > > > +/* Bitmask of the features supported by the device (host)
> > > > > + * (32 bits per set) - Read Only */
> > > > > +#define VIRTIO_MDEV_DEVICE_FEATURES	0x010
> > > > > +
> > > > > +/* Device (host) features set selector - Write Only */
> > > > > +#define VIRTIO_MDEV_DEVICE_FEATURES_SEL	0x014
> > > > > +
> > > > > +/* Bitmask of features activated by the driver (guest)
> > > > > + * (32 bits per set) - Write Only */
> > > > > +#define VIRTIO_MDEV_DRIVER_FEATURES	0x020
> > > > > +
> > > > > +/* Activated features set selector - Write Only */
> > > > > +#define VIRTIO_MDEV_DRIVER_FEATURES_SEL	0x024
> > > > > +
> > > > > +/* Queue selector - Write Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_SEL		0x030
> > > > > +
> > > > > +/* Maximum size of the currently selected queue - Read Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_NUM_MAX	0x034
> > > > > +
> > > > > +/* Queue size for the currently selected queue - Write Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_NUM		0x038
> > > > > +
> > > > > +/* Ready bit for the currently selected queue - Read Write */
> > > > > +#define VIRTIO_MDEV_QUEUE_READY		0x044
> > > > Is this same as started?
> > > 
> > > Do you mean "status"?
> > I really meant "enabled", didn't remember the correct name.
> > As in:  VIRTIO_PCI_COMMON_Q_ENABLE
> 
> 
> Yes, it's the same.
> 
> Thanks
> 
> 
> > 
> > > > > +
> > > > > +/* Alignment of virtqueue - Read Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_ALIGN		0x048
> > > > > +
> > > > > +/* Queue notifier - Write Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_NOTIFY	0x050
> > > > > +
> > > > > +/* Device status register - Read Write */
> > > > > +#define VIRTIO_MDEV_STATUS		0x060
> > > > > +
> > > > > +/* Selected queue's Descriptor Table address, 64 bits in two halves */
> > > > > +#define VIRTIO_MDEV_QUEUE_DESC_LOW	0x080
> > > > > +#define VIRTIO_MDEV_QUEUE_DESC_HIGH	0x084
> > > > > +
> > > > > +/* Selected queue's Available Ring address, 64 bits in two halves */
> > > > > +#define VIRTIO_MDEV_QUEUE_AVAIL_LOW	0x090
> > > > > +#define VIRTIO_MDEV_QUEUE_AVAIL_HIGH	0x094
> > > > > +
> > > > > +/* Selected queue's Used Ring address, 64 bits in two halves */
> > > > > +#define VIRTIO_MDEV_QUEUE_USED_LOW	0x0a0
> > > > > +#define VIRTIO_MDEV_QUEUE_USED_HIGH	0x0a4
> > > > > +
> > > > > +/* Configuration atomicity value */
> > > > > +#define VIRTIO_MDEV_CONFIG_GENERATION	0x0fc
> > > > > +
> > > > > +/* The config space is defined by each driver as
> > > > > + * the per-driver configuration space - Read Write */
> > > > > +#define VIRTIO_MDEV_CONFIG		0x100
> > > > Mixing device and generic config space is what virtio pci did,
> > > > caused lots of problems with extensions.
> > > > It would be better to reserve much more space.
> > > 
> > > I see, will do this.
> > > 
> > > Thanks
> > > 
> > > 
> > > > 
> > > > > +
> > > > > +#endif
> > > > > +
> > > > > +
> > > > > +/* Ready bit for the currently selected queue - Read Write */
> > > > > -- 
> > > > > 2.19.1

^ permalink raw reply

* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Xin Long @ 2019-09-11  9:38 UTC (permalink / raw)
  To: David Laight
  Cc: network dev, linux-sctp@vger.kernel.org, Marcelo Ricardo Leitner,
	Neil Horman, davem@davemloft.net
In-Reply-To: <CADvbK_dcGXPmO+wwwCvcsoGYPv+sdpw2b0cGuen-QPuxNcEcpQ@mail.gmail.com>

On Wed, Sep 11, 2019 at 5:21 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Wed, Sep 11, 2019 at 5:03 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Xin Long [mailto:lucien.xin@gmail.com]
> > > Sent: 11 September 2019 09:52
> > > On Tue, Sep 10, 2019 at 9:19 PM David Laight <David.Laight@aculab.com> wrote:
> > > >
> > > > From: Xin Long
> > > > > Sent: 09 September 2019 08:57
> > > > > Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
> > > > > Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
> > > > > added to allow a user to change ps_retrans per sock/asoc/transport, as
> > > > > other 2 paddrthlds: pf_retrans, pathmaxrxt.
> > > > >
> > > > > Note that ps_retrans is not allowed to be greater than pf_retrans.
> > > > >
> > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > ---
> > > > >  include/uapi/linux/sctp.h |  1 +
> > > > >  net/sctp/socket.c         | 10 ++++++++++
> > > > >  2 files changed, 11 insertions(+)
> > > > >
> > > > > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > > > > index a15cc28..dfd81e1 100644
> > > > > --- a/include/uapi/linux/sctp.h
> > > > > +++ b/include/uapi/linux/sctp.h
> > > > > @@ -1069,6 +1069,7 @@ struct sctp_paddrthlds {
> > > > >       struct sockaddr_storage spt_address;
> > > > >       __u16 spt_pathmaxrxt;
> > > > >       __u16 spt_pathpfthld;
> > > > > +     __u16 spt_pathcpthld;
> > > > >  };
> > > > >
> > > > >  /*
> > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > > > index 5e2098b..5b9774d 100644
> > > > > --- a/net/sctp/socket.c
> > > > > +++ b/net/sctp/socket.c
> > > > > @@ -3954,6 +3954,9 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
> > > >
> > > > This code does:
> > > >         if (optlen < sizeof(struct sctp_paddrthlds))
> > > >                 return -EINVAL;
> > > here will become:
> > >
> > >         if (optlen >= sizeof(struct sctp_paddrthlds)) {
> > >                 optlen = sizeof(struct sctp_paddrthlds);
> > >         } else if (optlen >= ALIGN(offsetof(struct sctp_paddrthlds,
> > >                                             spt_pathcpthld), 4))
> > >                 optlen = ALIGN(offsetof(struct sctp_paddrthlds,
> > >                                         spt_pathcpthld), 4);
> > >                 val.spt_pathcpthld = 0xffff;
> > >         else {
> > >                 return -EINVAL;
> > >         }
> >
> > Hmmm...
> > If the kernel has to default 'val.spt_pathcpthld = 0xffff'
> > then recompiling an existing application with the new uapi
> > header is going to lead to very unexpected behaviour.
> >
> > The best you can hope for is that the application memset the
> > structure to zero.
> > But more likely it is 'random' on-stack data.
> 0xffff is a value to disable the feature 'Primary Path Switchover'.
> you're right that user might set it to zero unexpectly with their
> old application rebuilt.
>
> A safer way is to introduce "sysctl net.sctp.ps_retrans", it won't
> matter if users set spt_pathcpthld properly when they're not aware
> of this feature, like "sysctl net.sctp.pF_retrans". Looks better?
Sorry for confusing,  "sysctl net.sctp.ps_retrans" is already there
(its value is 0xffff by default),
we just need to do this in sctp_setsockopt_paddr_thresholds():

        if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval,
                           optlen))
                return -EFAULT;

        if (sock_net(sk)->sctp.ps_retrans == 0xffff)
                val.spt_pathcpthld = 0xffff;

        if (val.spt_pathpfthld > val.spt_pathcpthld)
                return -EINVAL;

>
> >
> >         David
> >
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Russell King - ARM Linux admin @ 2019-09-11  9:39 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Dmitry Torokhov, Linus Walleij, Mika Westerberg, linux-kernel,
	linux-gpio, Andrew Lunn, David S. Miller, Florian Fainelli,
	Heiner Kallweit, netdev
In-Reply-To: <20190911092514.GM2680@smile.fi.intel.com>

On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> > works with arbitrary firmware node.
> 
> I'm wondering if it's possible to step forward and replace
> fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
> in other cases in this series.

No, those require a struct device, but we have none.  There are network
drivers where there is a struct device for the network complex, but only
DT nodes for the individual network interfaces.  So no, gpiod_* really
doesn't work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH 0/7] net: dsa: mv88e6xxx: features to handle network storms
From: Robert Beckett @ 2019-09-11  9:43 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Ido Schimmel,
	Jiri Pirko, bob.beckett
In-Reply-To: <545d6473-848f-3194-02a6-011b7c89a2ca@gmail.com>

On Tue, 2019-09-10 at 09:49 -0700, Florian Fainelli wrote:
> +Ido, Jiri,
> 
> On 9/10/19 8:41 AM, Robert Beckett wrote:
> > This patch-set adds support for some features of the Marvell switch
> > chips that can be used to handle packet storms.
> > 
> > The rationale for this was a setup that requires the ability to
> > receive
> > traffic from one port, while a packet storm is occuring on another
> > port
> > (via an external switch with a deliberate loop). This is needed to
> > ensure vital data delivery from a specific port, while mitigating
> > any
> > loops or DoS that a user may introduce on another port (can't
> > guarantee
> > sensible users).
> 
> The use case is reasonable, but the implementation is not really. You
> are using Device Tree which is meant to describe hardware as a policy
> holder for setting up queue priorities and likewise for queue
> scheduling.
> 
> The tool that should be used for that purpose is tc and possibly an
> appropriately offloaded queue scheduler in order to map the desired
> scheduling class to what the hardware supports.

Thanks for the review and tip about tc. Im currently not familiar with
that tool. Ill investigate it as an alternative approach.

> 
> Jiri, Ido, how do you guys support this with mlxsw?
> 
> > 
> > [patch 1/7] configures auto negotiation for CPU ports connected
> > with
> > phys to enable pause frame propogation.
> > 
> > [patch 2/7] allows setting of port's default output queue priority
> > for
> > any ingressing packets on that port.
> > 
> > [patch 3/7] dt-bindings for patch 2.
> > 
> > [patch 4/7] allows setting of a port's queue scheduling so that it
> > can
> > prioritise egress of traffic routed from high priority ports.
> > 
> > [patch 5/7] dt-bindings for patch 4.
> > 
> > [patch 6/7] allows ports to rate limit their egress. This can be
> > used to
> > stop the host CPU from becoming swamped by packet delivery and
> > exhasting
> > descriptors.
> > 
> > [patch 7/7] dt-bindings for patch 6.
> > 
> > 
> > Robert Beckett (7):
> >   net/dsa: configure autoneg for CPU port
> >   net: dsa: mv88e6xxx: add ability to set default queue priorities
> > per
> >     port
> >   dt-bindings: mv88e6xxx: add ability to set default queue
> > priorities
> >     per port
> >   net: dsa: mv88e6xxx: add ability to set queue scheduling
> >   dt-bindings: mv88e6xxx: add ability to set queue scheduling
> >   net: dsa: mv88e6xxx: add egress rate limiting
> >   dt-bindings: mv88e6xxx: add egress rate limiting
> > 
> >  .../devicetree/bindings/net/dsa/marvell.txt   |  38 +++++
> >  drivers/net/dsa/mv88e6xxx/chip.c              | 122 ++++++++++++
> > ---
> >  drivers/net/dsa/mv88e6xxx/chip.h              |   5 +-
> >  drivers/net/dsa/mv88e6xxx/port.c              | 140
> > +++++++++++++++++-
> >  drivers/net/dsa/mv88e6xxx/port.h              |  24 ++-
> >  include/dt-bindings/net/dsa-mv88e6xxx.h       |  22 +++
> >  net/dsa/port.c                                |  10 ++
> >  7 files changed, 327 insertions(+), 34 deletions(-)
> >  create mode 100644 include/dt-bindings/net/dsa-mv88e6xxx.h
> > 
> 
> 


^ permalink raw reply

* [PATCH] vhost: make sure log_num < in_num
From: > @ 2019-09-11  9:44 UTC (permalink / raw)
  To: mst, jasowang
  Cc: kvm, virtualization, netdev, linux-kernel, lidongchen, yongduan,
	ruippan

From: yongduan <yongduan@tencent.com>

The code assumes log_num < in_num everywhere, and that is true as long as
in_num is incremented by descriptor iov count, and log_num by 1. However
this breaks if there's a zero sized descriptor.

As a result, if a malicious guest creates a vring desc with desc.len = 0,
it may cause the host kernel to crash by overflowing the log array. This
bug can be triggered during the VM migration.

There's no need to log when desc.len = 0, so just don't increment log_num
in this case.

Fixes: 3a4d5c94e959 ("vhost_net: a kernel-level virtio server")
Reviewed-by: Lidong Chen <lidongchen@tencent.com>
Signed-off-by: ruippan <ruippan@tencent.com>
Signed-off-by: yongduan <yongduan@tencent.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
---
 drivers/vhost/vhost.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 5dc174a..36ca2cf 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2178,7 +2178,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
 		/* If this is an input descriptor, increment that count. */
 		if (access == VHOST_ACCESS_WO) {
 			*in_num += ret;
-			if (unlikely(log)) {
+			if (unlikely(log && ret)) {
 				log[*log_num].addr = vhost64_to_cpu(vq, desc.addr);
 				log[*log_num].len = vhost32_to_cpu(vq, desc.len);
 				++*log_num;
@@ -2319,7 +2319,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
 			/* If this is an input descriptor,
 			 * increment that count. */
 			*in_num += ret;
-			if (unlikely(log)) {
+			if (unlikely(log && ret)) {
 				log[*log_num].addr = vhost64_to_cpu(vq, desc.addr);
 				log[*log_num].len = vhost32_to_cpu(vq, desc.len);
 				++*log_num;
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH 0/7] net: dsa: mv88e6xxx: features to handle network storms
From: Robert Beckett @ 2019-09-11  9:46 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, Andrew Lunn, Florian Fainelli, David S. Miller,
	bob.beckett
In-Reply-To: <20190910131951.GM32337@t480s.localdomain>

On Tue, 2019-09-10 at 13:19 -0400, Vivien Didelot wrote:
> Hi Robert,
> 
> On Tue, 10 Sep 2019 16:41:46 +0100, Robert Beckett <
> bob.beckett@collabora.com> wrote:
> > This patch-set adds support for some features of the Marvell switch
> > chips that can be used to handle packet storms.
> > 
> > The rationale for this was a setup that requires the ability to
> > receive
> > traffic from one port, while a packet storm is occuring on another
> > port
> > (via an external switch with a deliberate loop). This is needed to
> > ensure vital data delivery from a specific port, while mitigating
> > any
> > loops or DoS that a user may introduce on another port (can't
> > guarantee
> > sensible users).
> > 
> > [patch 1/7] configures auto negotiation for CPU ports connected
> > with
> > phys to enable pause frame propogation.
> > 
> > [patch 2/7] allows setting of port's default output queue priority
> > for
> > any ingressing packets on that port.
> > 
> > [patch 3/7] dt-bindings for patch 2.
> > 
> > [patch 4/7] allows setting of a port's queue scheduling so that it
> > can
> > prioritise egress of traffic routed from high priority ports.
> > 
> > [patch 5/7] dt-bindings for patch 4.
> > 
> > [patch 6/7] allows ports to rate limit their egress. This can be
> > used to
> > stop the host CPU from becoming swamped by packet delivery and
> > exhasting
> > descriptors.
> > 
> > [patch 7/7] dt-bindings for patch 6.
> > 
> > 
> > Robert Beckett (7):
> >   net/dsa: configure autoneg for CPU port
> >   net: dsa: mv88e6xxx: add ability to set default queue priorities
> > per
> >     port
> >   dt-bindings: mv88e6xxx: add ability to set default queue
> > priorities
> >     per port
> >   net: dsa: mv88e6xxx: add ability to set queue scheduling
> >   dt-bindings: mv88e6xxx: add ability to set queue scheduling
> >   net: dsa: mv88e6xxx: add egress rate limiting
> >   dt-bindings: mv88e6xxx: add egress rate limiting
> > 
> >  .../devicetree/bindings/net/dsa/marvell.txt   |  38 +++++
> >  drivers/net/dsa/mv88e6xxx/chip.c              | 122 ++++++++++++
> > ---
> >  drivers/net/dsa/mv88e6xxx/chip.h              |   5 +-
> >  drivers/net/dsa/mv88e6xxx/port.c              | 140
> > +++++++++++++++++-
> >  drivers/net/dsa/mv88e6xxx/port.h              |  24 ++-
> >  include/dt-bindings/net/dsa-mv88e6xxx.h       |  22 +++
> >  net/dsa/port.c                                |  10 ++
> >  7 files changed, 327 insertions(+), 34 deletions(-)
> >  create mode 100644 include/dt-bindings/net/dsa-mv88e6xxx.h
> 
> Feature series targeting netdev must be prefixed "PATCH net-next". As

Thanks for the info. Out of curiosity, where should I have gleaned this
info from? This is my first contribution to netdev, so I wasnt familiar
with the etiquette.

> this approach was a PoC, sending it as "RFC net-next" would be even
> more
> appropriate.
> 
> 
> Thank you,
> 
> 	Vivien


^ permalink raw reply

* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Andy Shevchenko @ 2019-09-11  9:46 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Dmitry Torokhov, Linus Walleij, Mika Westerberg, linux-kernel,
	linux-gpio, Andrew Lunn, David S. Miller, Florian Fainelli,
	Heiner Kallweit, netdev
In-Reply-To: <20190911093914.GT13294@shell.armlinux.org.uk>

On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin wrote:
> On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> > > works with arbitrary firmware node.
> > 
> > I'm wondering if it's possible to step forward and replace
> > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
> > in other cases in this series.
> 
> No, those require a struct device, but we have none.  There are network
> drivers where there is a struct device for the network complex, but only
> DT nodes for the individual network interfaces.  So no, gpiod_* really
> doesn't work.

In the following patch the node is derived from struct device. So, I believe
some cases can be handled differently.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Russell King - ARM Linux admin @ 2019-09-11  9:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Dmitry Torokhov, Linus Walleij, Mika Westerberg, linux-kernel,
	linux-gpio, Andrew Lunn, David S. Miller, Florian Fainelli,
	Heiner Kallweit, netdev
In-Reply-To: <20190911094619.GN2680@smile.fi.intel.com>

On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote:
> > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> > > > works with arbitrary firmware node.
> > > 
> > > I'm wondering if it's possible to step forward and replace
> > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
> > > in other cases in this series.
> > 
> > No, those require a struct device, but we have none.  There are network
> > drivers where there is a struct device for the network complex, but only
> > DT nodes for the individual network interfaces.  So no, gpiod_* really
> > doesn't work.
> 
> In the following patch the node is derived from struct device. So, I believe
> some cases can be handled differently.

phylink is not passed a struct device - it has no knowledge what the
parent device is.

In any case, I do not have "the following patch".

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Dmitry Torokhov @ 2019-09-11  9:51 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Russell King - ARM Linux admin, Linus Walleij, Mika Westerberg,
	linux-kernel, linux-gpio, Andrew Lunn, David S. Miller,
	Florian Fainelli, Heiner Kallweit, netdev
In-Reply-To: <20190911094619.GN2680@smile.fi.intel.com>

On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote:
> > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> > > > works with arbitrary firmware node.
> > > 
> > > I'm wondering if it's possible to step forward and replace
> > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
> > > in other cases in this series.
> > 
> > No, those require a struct device, but we have none.  There are network
> > drivers where there is a struct device for the network complex, but only
> > DT nodes for the individual network interfaces.  So no, gpiod_* really
> > doesn't work.
> 
> In the following patch the node is derived from struct device. So, I believe
> some cases can be handled differently.

If we are willing to sacrifice the custom label for the GPIO that
fwnode_gpiod_get_index() allows us to set, then there are several
drivers that could actually use gpiod_get() API.

This is up to the dirver's maintainers...

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
From: Jason Wang @ 2019-09-11  9:53 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kvm, virtualization, netdev, linux-kernel, kwankhede,
	alex.williamson, cohuck, tiwei.bie, maxime.coquelin,
	cunming.liang, zhihong.wang, rob.miller, idos, xiao.w.wang,
	haotian.wang
In-Reply-To: <20190911053502-mutt-send-email-mst@kernel.org>


On 2019/9/11 下午5:36, Michael S. Tsirkin wrote:
> On Wed, Sep 11, 2019 at 10:38:39AM +0800, Jason Wang wrote:
>> On 2019/9/10 下午9:52, Michael S. Tsirkin wrote:
>>> On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote:
>>>> On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
>>>>>> +#ifndef _LINUX_VIRTIO_MDEV_H
>>>>>> +#define _LINUX_VIRTIO_MDEV_H
>>>>>> +
>>>>>> +#include <linux/interrupt.h>
>>>>>> +#include <linux/vringh.h>
>>>>>> +#include <uapi/linux/virtio_net.h>
>>>>>> +
>>>>>> +/*
>>>>>> + * Ioctls
>>>>>> + */
>>>>> Pls add a bit more content here. It's redundant to state these
>>>>> are ioctls. Much better to document what does each one do.
>>>> Ok.
>>>>
>>>>
>>>>>> +
>>>>>> +struct virtio_mdev_callback {
>>>>>> +	irqreturn_t (*callback)(void *);
>>>>>> +	void *private;
>>>>>> +};
>>>>>> +
>>>>>> +#define VIRTIO_MDEV 0xAF
>>>>>> +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
>>>>>> +					 struct virtio_mdev_callback)
>>>>>> +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
>>>>>> +					struct virtio_mdev_callback)
>>>>> Function pointer in an ioctl parameter? How does this ever make sense?
>>>> I admit this is hacky (casting).
>>>>
>>>>
>>>>> And can't we use a couple of registers for this, and avoid ioctls?
>>>> Yes, how about something like interrupt numbers for each virtqueue and
>>>> config?
>>> Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then?
>>
>> You mean something like VIRTIO_PCI_COMMON_Q_MSIX? Then it becomes a PCI
>> transport in fact. And using either MSIX or irq number is actually another
>> layer of indirection. So I think we can just write callback function and
>> parameter through registers.
> I just realized, all these registers are just encoded so you
> can pass stuff through read/write. But it can instead be
> just a normal C function call with no messy encoding.
> So why do we want to do this encoding?


Just because it was easier to start as a POC since mdev_parent_ops is 
the only way to communicate between mdev driver and mdev device right 
now. We can invent private ops besides mdev_parent_ops, e.g a private 
pointer in mdev_parent_ops. I can try this in next version.

Thanks


^ permalink raw reply

* Re: [PATCH 1/7] net/dsa: configure autoneg for CPU port
From: Robert Beckett @ 2019-09-11  9:54 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn; +Cc: netdev, Vivien Didelot, David S. Miller
In-Reply-To: <ad302835a98ca5abc7ac88b3caad64867e33ee70.camel@collabora.com>

On Wed, 2019-09-11 at 10:16 +0100, Robert Beckett wrote:
> On Tue, 2019-09-10 at 11:29 -0700, Florian Fainelli wrote:
> > On 9/10/19 11:26 AM, Andrew Lunn wrote:
> > > On Tue, Sep 10, 2019 at 04:41:47PM +0100, Robert Beckett wrote:
> > > > This enables us to negoatiate pause frame transmission to
> > > > prioritise
> > > > packet delivery over throughput.
> > > 
> > > I don't think we can unconditionally enable this. It is a big
> > > behaviour change, and it is likely to break running systems. It
> > > has
> > > affects on QoS, packet prioritisation, etc.
> > > 
> > > I think there needs to be a configuration knob. But
> > > unfortunately,
> > > i
> > > don't know of a good place to put this knob. The switch CPU port
> > > is
> > > not visible in any way.
> > 
> > Broadcast storm suppression is to be solved at ingress, not on the
> > CPU
> > port, once this lands on the CPU port, it's game over already.
> 
> It is not just for broadcast storm protection. The original issue
> that
> made me look in to all of this turned out to be rx descritor ring
> buffer exhaustion due to the CPU not being able to keep up with
> packet
> reception.
> 
> Although the simple repro case for it is a broadcast storm, this
> could
> happen with many legitimate small packets, and the correct way to
> handle it seems to be pause frames, though I am not traditionally a
> network programmer, so my knowledge may be incorrect. Please advise
> if
> you know of a better way to handle that.
> 
> Fundamentally, with a phy to phy CPU connection, the CPU MAC may well
> wish to enable pause frames for various reasons, so we should strive
> to
> handle that I think.
> 

As an aside, do any of you have experience of trying to enable PIRL on
the Marvell switches? The first thing I tried was configuring it for
packet number based (rather than byte count based) input rate limiting,
but it never seemed to have any effect even at extreme values that
should in theory have greatly limited the number of packets allowed to
ingress.

After investigating the root cause and finding it was due to the CPU's
inability to process the received packets quickly enough, pause frames
and port prioritization seemed like the correct fix anyway.


^ permalink raw reply

* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Dmitry Torokhov @ 2019-09-11  9:55 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Andy Shevchenko, Linus Walleij, Mika Westerberg, linux-kernel,
	linux-gpio, Andrew Lunn, David S. Miller, Florian Fainelli,
	Heiner Kallweit, netdev
In-Reply-To: <20190911094929.GV13294@shell.armlinux.org.uk>

On Wed, Sep 11, 2019 at 10:49:29AM +0100, Russell King - ARM Linux admin wrote:
> On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin wrote:
> > > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> > > > > works with arbitrary firmware node.
> e > > 
> > > > I'm wondering if it's possible to step forward and replace
> > > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
> > > > in other cases in this series.
> > > 
> > > No, those require a struct device, but we have none.  There are network
> > > drivers where there is a struct device for the network complex, but only
> > > DT nodes for the individual network interfaces.  So no, gpiod_* really
> > > doesn't work.
> > 
> > In the following patch the node is derived from struct device. So, I believe
> > some cases can be handled differently.
> 
> phylink is not passed a struct device - it has no knowledge what the
> parent device is.
> 
> In any case, I do not have "the following patch".

Andy is talking about this one:

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index ce940871331e..9ca51d678123 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -46,8 +46,8 @@ static int mdiobus_register_gpiod(struct mdio_device *mdiodev)

        /* Deassert the optional reset signal */
        if (mdiodev->dev.of_node)
-               gpiod = fwnode_get_named_gpiod(&mdiodev->dev.of_node->fwnode,
-                                              "reset-gpios", 0,
                                               GPIOD_OUT_LOW,
+               gpiod = fwnode_gpiod_get_index(&mdiodev->dev.of_node->fwnode,
+                                              "reset", 0, GPIOD_OUT_LOW,
                                               "PHY reset");
Here if we do not care about "PHY reset" label, we could use
gpiod_get(&mdiodev->dev, "reset", GPIOD_OUT_LOW).

Thanks.

-- 
Dmitry

^ permalink raw reply related

* WARNING at net/mac80211/sta_info.c:1057 (__sta_info_destroy_part2())
From: Linus Torvalds @ 2019-09-11 10:05 UTC (permalink / raw)
  To: Johannes Berg, David S. Miller
  Cc: linux-wireless, Netdev, Linux List Kernel Mailing

So I'm at LCA, reading email, using my laptop more than I normally do,
and with different networking than I normally do.

And I just had a 802.11 WARN_ON() trigger, followed by essentially a
dead machine due to some lock held (maybe rtnl_lock).

It's possible that the lock held thing happened before, and is the
_reason_ for the delay, I don't know. I had to reboot the machine, but
I gathered as much information as made sense and was obvious before I
did so. That's appended.

                 Linus

---
Selected dmesg from the WARN_ON_ONCE() and just before, followed by
info from "sysrq-w":

Previous resume looks normal:

   PM: suspend exit
   ath10k_pci 0000:02:00.0: UART prints enabled
   ath10k_pci 0000:02:00.0: unsupported HTC service id: 1536
   wlp2s0: authenticate with 54:ec:2f:05:70:2c
   wlp2s0: send auth to 54:ec:2f:05:70:2c (try 1/3)
   wlp2s0: send auth to 54:ec:2f:05:70:2c (try 2/3)
   wlp2s0: send auth to 54:ec:2f:05:70:2c (try 3/3)
   wlp2s0: authentication with 54:ec:2f:05:70:2c timed out
   wlp2s0: authenticate with 54:ec:2f:05:70:28
   wlp2s0: send auth to 54:ec:2f:05:70:28 (try 1/3)
   wlp2s0: authenticated
   wlp2s0: associate with 54:ec:2f:05:70:28 (try 1/3)
   wlp2s0: RX AssocResp from 54:ec:2f:05:70:28 (capab=0x1431 status=0 aid=1)
   wlp2s0: associated
   wlp2s0: Limiting TX power to 20 (20 - 0) dBm as advertised by
54:ec:2f:05:70:28
   ath: EEPROM regdomain: 0x826c
   ath: EEPROM indicates we should expect a country code
   ath: doing EEPROM country->regdmn map search
   ath: country maps to regdmn code: 0x37
   ath: Country alpha2 being used: PT
   ath: Regpair used: 0x37
   ath: regdomain 0x826c dynamically updated by country element
   IPv6: ADDRCONF(NETDEV_CHANGE): wlp2s0: link becomes ready
   wlp2s0: disconnect from AP 54:ec:2f:05:70:28 for new auth to
54:ec:2f:05:70:2c
   wlp2s0: authenticate with 54:ec:2f:05:70:2c
   wlp2s0: send auth to 54:ec:2f:05:70:2c (try 1/3)
   wlp2s0: authenticated
   wlp2s0: associate with 54:ec:2f:05:70:2c (try 1/3)
   wlp2s0: RX ReassocResp from 54:ec:2f:05:70:2c (capab=0x1011 status=0 aid=2)
   wlp2s0: associated
   ath: EEPROM regdomain: 0x826c
   ath: EEPROM indicates we should expect a country code
   ath: doing EEPROM country->regdmn map search
   ath: country maps to regdmn code: 0x37
   ath: Country alpha2 being used: PT
   ath: Regpair used: 0x37
   ath: regdomain 0x826c dynamically updated by country element
   wlp2s0: Limiting TX power to 23 (23 - 0) dBm as advertised by
54:ec:2f:05:70:2c

Another _almost_ successful suspend/resume:

   PM: suspend entry (s2idle)
   Filesystems sync: 0.028 seconds
   Freezing user space processes ... (elapsed 0.126 seconds) done.
   OOM killer disabled.
   Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
   printk: Suspending console(s) (use no_console_suspend to debug)
   wlp2s0: deauthenticating from 54:ec:2f:05:70:2c by local choice
(Reason: 3=DEAUTH_LEAVING)
   ath10k_pci 0000:02:00.0: UART prints enabled
   ath10k_pci 0000:02:00.0: unsupported HTC service id: 1536
   OOM killer enabled.
   Restarting tasks ... done.
   PM: suspend exit
   ath10k_pci 0000:02:00.0: UART prints enabled
   ath10k_pci 0000:02:00.0: unsupported HTC service id: 1536
   wlp2s0: authenticate with 54:ec:2f:05:70:2c
   wlp2s0: send auth to 54:ec:2f:05:70:2c (try 1/3)
   wlp2s0: authenticated
   wlp2s0: associate with 54:ec:2f:05:70:2c (try 1/3)
   wlp2s0: RX AssocResp from 54:ec:2f:05:70:2c (capab=0x1011 status=0 aid=2)
   wlp2s0: associated
   wlp2s0: Limiting TX power to 23 (23 - 0) dBm as advertised by
54:ec:2f:05:70:2c
   ath: EEPROM regdomain: 0x826c
   ath: EEPROM indicates we should expect a country code
   ath: doing EEPROM country->regdmn map search
   ath: country maps to regdmn code: 0x37
   ath: Country alpha2 being used: PT
   ath: Regpair used: 0x37
   ath: regdomain 0x826c dynamically updated by country element
   IPv6: ADDRCONF(NETDEV_CHANGE): wlp2s0: link becomes ready

I say _almost_, because I don't see the "No TX power limit" for the
country lookup (yes, Portugal) this time?

But wait!

... then 10+ minutes later:

   ath10k_pci 0000:02:00.0: wmi command 16387 timeout, restarting hardware
   ath10k_pci 0000:02:00.0: failed to set 5g txpower 23: -11
   ath10k_pci 0000:02:00.0: failed to setup tx power 23: -11
   ath10k_pci 0000:02:00.0: failed to recalc tx power: -11
   ath10k_pci 0000:02:00.0: failed to set inactivity time for vdev 0: -108
   ath10k_pci 0000:02:00.0: failed to setup powersave: -108

That certainly looks like something did try to set a power limit, but
eventually failed.

Immediately after that:

   wlp2s0: deauthenticating from 54:ec:2f:05:70:2c by local choice
(Reason: 3=DEAUTH_LEAVING)
   ath10k_pci 0000:02:00.0: failed to read hi_board_data address: -16
   ath10k_pci 0000:02:00.0: failed to receive initialized event from
target: 00000000
   ath10k_pci 0000:02:00.0: failed to receive initialized event from
target: 00000000
   ath10k_pci 0000:02:00.0: failed to wait for target init: -110
   ieee80211 phy0: Hardware restart was requested
   ath10k_pci 0000:02:00.0: failed to set inactivity time for vdev 0: -108
   ath10k_pci 0000:02:00.0: failed to setup powersave: -108
   ath10k_pci 0000:02:00.0: failed to set PS Mode 0 for vdev 0: -108
   ath10k_pci 0000:02:00.0: failed to setup powersave: -108
   ath10k_pci 0000:02:00.0: failed to setup ps on vdev 0: -108
   ath10k_pci 0000:02:00.0: failed to flush transmit queue (skip 1
ar-state 2): 5000
   ath10k_pci 0000:02:00.0: failed to delete peer 54:ec:2f:05:70:2c
for vdev 0: -108

and this then results in:

   WARNING: CPU: 4 PID: 1246 at net/mac80211/sta_info.c:1057
__sta_info_destroy_part2+0x147/0x150 [mac80211]
   Modules linked in: ccm fuse rfcomm xt_CHECKSUM xt_MASQUERADE tun
bridge stp llc nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT
ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_nat
ip6table_nat ip6table_mangle ip6table_raw ip6table_security
iptable_nat nf_nat iptable_mangle iptable_raw iptable_security
nf_conntrack nf_defrag_ipv6 libcrc32c nf_defrag_ipv4 ip_set nfnetlink
ebtable_filter ebtables ip6table_filter ip6_tables cmac bnep vfat fat
hid_multitouch iTCO_wdt iTCO_vendor_support snd_hda_codec_hdmi
snd_soc_skl snd_soc_hdac_hda snd_hda_ext_core snd_hda_codec_realtek
snd_soc_core snd_hda_codec_generic snd_soc_acpi_intel_match
snd_soc_acpi snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp
dell_laptop snd_hda_intel ledtrig_audio dell_wmi dell_smbios
x86_pkg_temp_thermal snd_hda_codec ath10k_pci intel_powerclamp
wmi_bmof dell_wmi_descriptor intel_wmi_thunderbolt snd_hwdep coretemp
ath10k_core snd_hda_core kvm_intel snd_seq ath snd_seq_device kvm
btusb btrtl
    intel_rapl_msr btbcm btintel mac80211 irqbypass snd_pcm bluetooth
intel_cstate dcdbas intel_uncore intel_rapl_perf joydev snd_timer snd
ecdh_generic i2c_i801 cfg80211 ecc idma64 soundcore rtsx_pci_ms
processor_thermal_device mei_me memstick rfkill libarc4 intel_lpss_pci
ucsi_acpi intel_soc_dts_iosf mei typec_ucsi intel_lpss
intel_pch_thermal intel_rapl_common typec wmi int3403_thermal
int340x_thermal_zone intel_hid sparse_keymap int3400_thermal acpi_pad
acpi_thermal_rel dm_crypt i915 crct10dif_pclmul crc32_pclmul
crc32c_intel rtsx_pci_sdmmc mmc_core i2c_algo_bit drm_kms_helper
syscopyarea sysfillrect sysimgblt nvme fb_sys_fops ghash_clmulni_intel
drm serio_raw nvme_core rtsx_pci i2c_hid pinctrl_cannonlake video
pinctrl_intel
   CPU: 4 PID: 1246 Comm: NetworkManager Not tainted
5.3.0-rc8-00004-g56037cadf604-dirty #5
   Hardware name: Dell Inc. XPS 13 9380/0KTW76, BIOS 1.5.0 06/03/2019
   RIP: 0010:__sta_info_destroy_part2+0x147/0x150 [mac80211]
   Code: bd 0c 01 00 00 00 0f 84 4e ff ff ff 45 31 c0 b9 01 00 00 00
48 89 ea 48 89 de 4c 89 e7 e8 e1 aa ff ff 85 c0 0f 84 30 ff ff ff <0f>
0b e9 29 ff ff ff 66 90 0f 1f 44 00 00 41 54 55 48 89 fd e8 40
   RSP: 0018:ffff98fc00daba90 EFLAGS: 00010282
   RAX: 00000000ffffff94 RBX: ffff8e1355ad2880 RCX: 0000000000000000
   RDX: ffff8e1355901ec0 RSI: 0000000000000000 RDI: ffff8e1357902e38
   RBP: ffff8e135bb08000 R08: 0000000000000000 R09: 0000000000000574
   R10: 000000000001cc5c R11: 0000000000000000 R12: ffff8e13579007a0
   R13: ffff8e1355ad2880 R14: 0000000000000001 R15: ffff8e1357900d58
   FS:  00007fbd99372bc0(0000) GS:ffff8e135e500000(0000) knlGS:0000000000000000
   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
   CR2: 00001cdbd7408000 CR3: 00000004708a2001 CR4: 00000000003606e0
   Call Trace:
    __sta_info_flush+0x125/0x170 [mac80211]
    ieee80211_set_disassoc+0xc2/0x590 [mac80211]
    ieee80211_mgd_deauth.cold+0x4a/0x1b8 [mac80211]
    cfg80211_mlme_deauth+0xb3/0x1d0 [cfg80211]
    ? startup_64+0x3/0x30
    cfg80211_mlme_down+0x66/0x90 [cfg80211]
    cfg80211_disconnect+0x129/0x1e0 [cfg80211]
    cfg80211_leave+0x27/0x40 [cfg80211]
    cfg80211_netdev_notifier_call+0x1a7/0x4e0 [cfg80211]
    ? ieee80211_iter_chan_contexts_atomic+0x3e/0x50 [mac80211]
    ? ath10k_monitor_recalc+0x38f/0x5d0 [ath10k_core]
    ? __schedule+0x143/0x660
    ? ath10k_config_ps+0x4e/0x70 [ath10k_core]
    ? inetdev_event+0x46/0x560
    notifier_call_chain+0x4c/0x70
    __dev_close_many+0x57/0x100
    dev_close_many+0x8d/0x140
    dev_close.part.0+0x44/0x70
    cfg80211_shutdown_all_interfaces+0x71/0xd0 [cfg80211]
    cfg80211_rfkill_set_block+0x22/0x30 [cfg80211]
    rfkill_set_block+0x92/0x140 [rfkill]
    rfkill_fop_write+0x11f/0x1c0 [rfkill]
    vfs_write+0xb6/0x1a0
    ksys_write+0xa7/0xe0
    do_syscall_64+0x59/0x1c0
    entry_SYSCALL_64_after_hwframe+0x44/0xa9
   RIP: 0033:0x7fbd9a36c92f
   Code: 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 19 fd ff ff 48 8b
54 24 18 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 01 00 00 00 0f 05 <48>
3d 00 f0 ff ff 77 2d 44 89 c7 48 89 44 24 08 e8 4c fd ff ff 48
   RSP: 002b:00007fff9a1ee220 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
   RAX: ffffffffffffffda RBX: 000055f18ef5eaad RCX: 00007fbd9a36c92f
   RDX: 0000000000000008 RSI: 00007fff9a1ee258 RDI: 0000000000000019
   RBP: 0000000000000019 R08: 0000000000000000 R09: 0000000000000001
   R10: 0000000000000000 R11: 0000000000000293 R12: 000055f190502090
   R13: 0000000000000000 R14: 0000000000000000 R15: 000055f1904ffbb0
   ---[ end trace 4d7c87877e20badd ]---
   ath10k_pci 0000:02:00.0: failed to recalculate rts/cts prot for vdev 0: -108
   ath10k_pci 0000:02:00.0: failed to set cts protection for vdev 0: -108

and it looks like it leaves some lock held.

Because after this, nothing network-related stops working, and the
machine eventually dies because workqueues don't make any progress (due
to rtnl_lock, I suspect):

   Workqueue: events_freezable ieee80211_restart_work [mac80211]
   Call Trace:
    schedule+0x39/0xa0
    schedule_preempt_disabled+0xa/0x10
    __mutex_lock.isra.0+0x263/0x4b0
    ieee80211_restart_work+0x54/0xe0 [mac80211]
    process_one_work+0x1cf/0x370
    worker_thread+0x4a/0x3c0
    kthread+0xfb/0x130
    ret_from_fork+0x35/0x40

   Workqueue: events disconnect_work [cfg80211]
   Call Trace:
    schedule+0x39/0xa0
    schedule_preempt_disabled+0xa/0x10
    __mutex_lock.isra.0+0x263/0x4b0
    disconnect_work+0x12/0xd0 [cfg80211]
    process_one_work+0x1cf/0x370
    worker_thread+0x4a/0x3c0
    kthread+0xfb/0x130
    ret_from_fork+0x35/0x40
   kworker/3:0     D    0  6313      2 0x80004000


   Workqueue: events linkwatch_event
   Call Trace:
    schedule+0x39/0xa0
    schedule_preempt_disabled+0xa/0x10
    __mutex_lock.isra.0+0x263/0x4b0
    linkwatch_event+0xa/0x30
    process_one_work+0x1cf/0x370
    worker_thread+0x4a/0x3c0
    kthread+0xfb/0x130
    ret_from_fork+0x35/0x40

   ping            D    0  6619   6568 0x80000002
   Call Trace:
    schedule+0x39/0xa0
    schedule_preempt_disabled+0xa/0x10
    __mutex_lock.isra.0+0x263/0x4b0
    ip6mr_sk_done+0x2e/0xd0
    rawv6_close+0x19/0x30
    inet_release+0x34/0x60
    __sock_release+0x3d/0xa0
    sock_close+0x11/0x20
    __fput+0xc1/0x250
    task_work_run+0x81/0xa0
    do_exit+0x2e5/0xaa0
    do_group_exit+0x3a/0x90
    __x64_sys_exit_group+0x14/0x20
    do_syscall_64+0x59/0x1c0
    entry_SYSCALL_64_after_hwframe+0x44/0xa9

   gdbus           D    0  6629   1949 0x80004000
   Call Trace:
    schedule+0x39/0xa0
    do_exit+0x1da/0xaa0
    do_group_exit+0x3a/0x90
    get_signal+0x14f/0x830
    do_signal+0x36/0x620
    exit_to_usermode_loop+0x6f/0xf0
    do_syscall_64+0x17d/0x1c0
    entry_SYSCALL_64_after_hwframe+0x44/0xa9

^ permalink raw reply

* RE: [RFC PATCH 3/3] Enable ptp_kvm for arm64
From: Jianyong Wu (Arm Technology China) @ 2019-09-11 10:06 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: netdev@vger.kernel.org, pbonzini@redhat.com,
	sean.j.christopherson@intel.com, richardcochran@gmail.com,
	Mark Rutland, Will Deacon, Suzuki Poulose,
	linux-kernel@vger.kernel.org, Steve Capper,
	Kaly Xin (Arm Technology China), Justin He (Arm Technology China)
In-Reply-To: <HE1PR0801MB16769814863B26F3B5DC708FF4B60@HE1PR0801MB1676.eurprd08.prod.outlook.com>

Hi Marc,

I think there are three points for the migration issue of ptp_kvm, where a VM using ptp_kvm migrates to a host without ptp_kvm support.

First: how does it impact the VM having migrated?
I run a VM with ptp_kvm support in guest but not support in host. the ptp0 will return 0 when get time from it which can't pass the check
of chrony, then chrony will choose another clocksource.  From this point, VM will only get lost in precision of time sync.

Second: how to check the failure of the ptp kvm service
when there is no ptp kvm service, hypercall will go into default ops, so we can check the return value which can inform us the failure.

Third: how to inform VMM
There is ioctl cmd call "KVM_CHECK_EXTENSION" in kvm, which may do that thing. Accordingly, qemu should be offered the support which will block us.
We can try to add this support in kvm but we are not sure the response from qemu side.

WDYT?

Jianyong Wu
Thanks

> -----Original Message-----
> From: Jianyong Wu (Arm Technology China)
> Sent: Tuesday, September 10, 2019 6:29 PM
> To: Marc Zyngier <maz@kernel.org>
> Cc: netdev@vger.kernel.org; pbonzini@redhat.com;
> sean.j.christopherson@intel.com; richardcochran@gmail.com; Mark Rutland
> <Mark.Rutland@arm.com>; Will Deacon <Will.Deacon@arm.com>; Suzuki
> Poulose <Suzuki.Poulose@arm.com>; linux-kernel@vger.kernel.org; Steve
> Capper <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> <Justin.He@arm.com>
> Subject: RE: [RFC PATCH 3/3] Enable ptp_kvm for arm64
>
> Hi Marc,
>
> > -----Original Message-----
> > From: Marc Zyngier <maz@kernel.org>
> > Sent: Monday, September 9, 2019 7:25 PM
> > To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>
> > Cc: netdev@vger.kernel.org; pbonzini@redhat.com;
> > sean.j.christopherson@intel.com; richardcochran@gmail.com; Mark
> > Rutland <Mark.Rutland@arm.com>; Will Deacon <Will.Deacon@arm.com>;
> > Suzuki Poulose <Suzuki.Poulose@arm.com>; linux-kernel@vger.kernel.org;
> > Steve Capper <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> > <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> > <Justin.He@arm.com>
> > Subject: Re: [RFC PATCH 3/3] Enable ptp_kvm for arm64
> >
> >
> > > > > > Other questions: how does this works with VM migration?
> > > > > > Specially when moving from a hypervisor that supports the
> > > > > > feature to one that
> > > > doesn't?
> > > > > >
> > > > > I think it won't solve the problem generated by VM migration and
> > > > > only for VMs in a single machine.  Ptp_kvm only works for VMs in
> > > > > the same machine.  But using ptp (not ptp_kvm) clock, all the
> > > > > machines in a low latency network environment can keep time sync
> > > > > in high precision, Then VMs move from one machine to another
> > > > > will obtain a high precision time sync.
> > > >
> > > > That's a problem. Migration must be possible from one host to
> > > > another, even if that means temporarily loosing some (or a lot of)
> > > > precision. The service must be discoverable from userspace on the
> > > > host so that the MVV can decie whether a migration is possible or not.
> > > >
> > > Don't worry, things will be not that bad.  ptp_kvm will not trouble
> > > the VM migration. This ptp_kvm is one clocksource of the clock pool
> > > for chrony. Chrony will choose the highest precision clock from the
> > > pool. If host does not support ptp_kvm, the ptp_kvm will not be
> > > chosen as the clocksouce of chrony.  We have roughly the same logic
> > > of implementation of ptp_kvm with x86, and ptp_kvm works well in
> > > x86.  so I think that will be the case for arm64.
> > >
> > > Maybe I miss your point, I have no idea of MVV and can't get related
> > > info from google.  Also I'm not clear of your last words of how to
> > > decide VM migration is possible?
> >
> > Sorry. s/MVV/VMM/. Basically userspace, such as QEMU.
> >
> > Here's an example: The guest runs on a PTP aware host, starts using
> > the PTP service and uses HVC calls to get its clock. We now migrate
> > the guest to a non PTP-aware host. The hypercalls are now going to
> > fail unexpectedly. Is that something that is acceptable? I don't think
> > it is. Once you've allowed a guest to use a service, this service
> > should be preserved. I'd be more confident if we gave to userspace the
> > indication that the hypervisor supports PTP. Userspace can then decide
> whether to perform migration or not.
> >
>
> It's really a point we should consider. let me check the behavior of chrony in
> this scenario first.
>
> Thanks
> Jianyong Wu
>
> > Thanks,
> >
> >     M.
> >
> > --
> > Jazz is not dead, it just smells funny.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply

* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Russell King - ARM Linux admin @ 2019-09-11 10:10 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Andy Shevchenko, Linus Walleij, Mika Westerberg, linux-kernel,
	linux-gpio, Andrew Lunn, David S. Miller, Florian Fainelli,
	Heiner Kallweit, netdev
In-Reply-To: <20190911095511.GB108334@dtor-ws>

On Wed, Sep 11, 2019 at 02:55:11AM -0700, Dmitry Torokhov wrote:
> On Wed, Sep 11, 2019 at 10:49:29AM +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote:
> > > On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin wrote:
> > > > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote:
> > > > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> > > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> > > > > > works with arbitrary firmware node.
> > e > > 
> > > > > I'm wondering if it's possible to step forward and replace
> > > > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
> > > > > in other cases in this series.
> > > > 
> > > > No, those require a struct device, but we have none.  There are network
> > > > drivers where there is a struct device for the network complex, but only
> > > > DT nodes for the individual network interfaces.  So no, gpiod_* really
> > > > doesn't work.
> > > 
> > > In the following patch the node is derived from struct device. So, I believe
> > > some cases can be handled differently.
> > 
> > phylink is not passed a struct device - it has no knowledge what the
> > parent device is.
> > 
> > In any case, I do not have "the following patch".
> 
> Andy is talking about this one:
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index ce940871331e..9ca51d678123 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -46,8 +46,8 @@ static int mdiobus_register_gpiod(struct mdio_device *mdiodev)
> 
>         /* Deassert the optional reset signal */
>         if (mdiodev->dev.of_node)
> -               gpiod = fwnode_get_named_gpiod(&mdiodev->dev.of_node->fwnode,
> -                                              "reset-gpios", 0,
>                                                GPIOD_OUT_LOW,
> +               gpiod = fwnode_gpiod_get_index(&mdiodev->dev.of_node->fwnode,
> +                                              "reset", 0, GPIOD_OUT_LOW,
>                                                "PHY reset");
> Here if we do not care about "PHY reset" label, we could use
> gpiod_get(&mdiodev->dev, "reset", GPIOD_OUT_LOW).

Here, you have a struct device, so yes, it's possible.

Referring back to my comment, notice that I said we have none for the
phylink case, so it's not possible there.

I'm not sure why Andy replied the way he did, unless he mis-read my
comment.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH V2 net-next 4/7] net: hns3: fix port setting handle for fibre port
From: Sergei Shtylyov @ 2019-09-11 10:16 UTC (permalink / raw)
  To: Huazhong Tan, davem
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski
In-Reply-To: <1568169639-43658-5-git-send-email-tanhuazhong@huawei.com>

Hello!

On 11.09.2019 5:40, Huazhong Tan wrote:

> From: Guangbin Huang <huangguangbin2@huawei.com>
> 
> For hardware doesn't support use specified speed and duplex

    Can't pasre that. "For hardware that does not support using", perhaps?

> to negotiate, it's unnecessary to check and modify the port
> speed and duplex for fibre port when autoneg is on.
> 
> Fixes: 22f48e24a23d ("net: hns3: add autoneg and change speed support for fibre port")
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
>   drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> index f5a681d..680c350 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> @@ -726,6 +726,12 @@ static int hns3_check_ksettings_param(const struct net_device *netdev,
>   	u8 duplex;
>   	int ret;
>   
> +	/* hw doesn't support use specified speed and duplex to negotiate,

    I can't parse that, did you mean "using"?

> +	 * unnecessary to check them when autoneg on.
> +	 */
> +	if (cmd->base.autoneg)
> +		return 0;
> +
>   	if (ops->get_ksettings_an_result) {
>   		ops->get_ksettings_an_result(handle, &autoneg, &speed, &duplex);
>   		if (cmd->base.autoneg == autoneg && cmd->base.speed == speed &&
> @@ -787,6 +793,15 @@ static int hns3_set_link_ksettings(struct net_device *netdev,
>   			return ret;
>   	}
>   
> +	/* hw doesn't support use specified speed and duplex to negotiate,

    Here too...

> +	 * ignore them when autoneg on.
> +	 */
> +	if (cmd->base.autoneg) {
> +		netdev_info(netdev,
> +			    "autoneg is on, ignore the speed and duplex\n");
> +		return 0;
> +	}
> +
>   	if (ops->cfg_mac_speed_dup_h)
>   		ret = ops->cfg_mac_speed_dup_h(handle, cmd->base.speed,
>   					       cmd->base.duplex);

MBR, Sergei

^ permalink raw reply

* Re: [PATCH V2 net-next 4/7] net: hns3: fix port setting handle for fibre port
From: Sergei Shtylyov @ 2019-09-11 10:17 UTC (permalink / raw)
  To: Huazhong Tan, davem
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski
In-Reply-To: <1568169639-43658-5-git-send-email-tanhuazhong@huawei.com>

Hello!

On 11.09.2019 5:40, Huazhong Tan wrote:

> From: Guangbin Huang <huangguangbin2@huawei.com>
> 
> For hardware doesn't support use specified speed and duplex

    Can't parse that. "For hardware that does not support using", perhaps?

> to negotiate, it's unnecessary to check and modify the port
> speed and duplex for fibre port when autoneg is on.
> 
> Fixes: 22f48e24a23d ("net: hns3: add autoneg and change speed support for fibre port")
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
>   drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> index f5a681d..680c350 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> @@ -726,6 +726,12 @@ static int hns3_check_ksettings_param(const struct net_device *netdev,
>   	u8 duplex;
>   	int ret;
>   
> +	/* hw doesn't support use specified speed and duplex to negotiate,

    I can't parse that, did you mean "using"?

> +	 * unnecessary to check them when autoneg on.
> +	 */
> +	if (cmd->base.autoneg)
> +		return 0;
> +
>   	if (ops->get_ksettings_an_result) {
>   		ops->get_ksettings_an_result(handle, &autoneg, &speed, &duplex);
>   		if (cmd->base.autoneg == autoneg && cmd->base.speed == speed &&
> @@ -787,6 +793,15 @@ static int hns3_set_link_ksettings(struct net_device *netdev,
>   			return ret;
>   	}
>   
> +	/* hw doesn't support use specified speed and duplex to negotiate,

    Here too...

> +	 * ignore them when autoneg on.
> +	 */
> +	if (cmd->base.autoneg) {
> +		netdev_info(netdev,
> +			    "autoneg is on, ignore the speed and duplex\n");
> +		return 0;
> +	}
> +
>   	if (ops->cfg_mac_speed_dup_h)
>   		ret = ops->cfg_mac_speed_dup_h(handle, cmd->base.speed,
>   					       cmd->base.duplex);

MBR, Sergei

^ permalink raw reply

* [PATCH iproute2-next] man: ss.8: add documentation for drop counter
From: Andrea Claudi @ 2019-09-11 10:19 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern

After commit 6df9c7a06a845 ("ss: add SK_MEMINFO_DROPS display") ss -m
displays also a drop counter for each socket.

This commit properly document it into the man page.

Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 man/man8/ss.8 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/man/man8/ss.8 b/man/man8/ss.8
index f428e60cc1949..023d771b17878 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -99,13 +99,13 @@ skmem:(r<rmem_alloc>,rb<rcv_buf>,t<wmem_alloc>,tb<snd_buf>,
 .br
 .RS
 .RS
-f<fwd_alloc>,w<wmem_queued>,
+f<fwd_alloc>,w<wmem_queued>,o<opt_mem>,
 .RE
 .RE
 .br
 .RS
 .RS
-o<opt_mem>,bl<back_log>)
+bl<back_log>,d<sock_drop>)
 .RE
 .RE
 .P
@@ -146,6 +146,10 @@ The memory used for the sk backlog queue. On a process context, if the
 process is receiving packet, and a new packet is received, it will be
 put into the sk backlog queue, so it can be received by the process
 immediately
+.P
+.TP
+.B <sock_drop>
+the number of packets dropped before they are de-multiplexed into the socket
 .RE
 .TP
 .B \-p, \-\-processes
-- 
2.21.0


^ permalink raw reply related

* Re: WARNING at net/mac80211/sta_info.c:1057 (__sta_info_destroy_part2())
From: Johannes Berg @ 2019-09-11 10:26 UTC (permalink / raw)
  To: Linus Torvalds, David S. Miller, Kalle Valo
  Cc: linux-wireless, Netdev, Linux List Kernel Mailing
In-Reply-To: <CAHk-=wgBuu8PiYpD7uWgxTSY8aUOJj6NJ=ivNQPYjAKO=cRinA@mail.gmail.com>

Hi,

> So I'm at LCA

When did LCA move to Portugal? ;-))

> , reading email, using my laptop more than I normally do,
> and with different networking than I normally do.
> 
> And I just had a 802.11 WARN_ON() trigger, followed by essentially a
> dead machine due to some lock held (maybe rtnl_lock).

yes, it's definitely stuck on the RTNL, all the lot of the workqueues
are trying to acquire it (linkwatch, ipv6, but oddly enough even the
mac80211 restart work).

> It's possible that the lock held thing happened before, and is the
> _reason_ for the delay, I don't know.

No, we do have the lock in the WARN_ON(), somewhere around
dev_close_many() it is acquired.

> Previous resume looks normal:
> [snip]
>    wlp2s0: Limiting TX power to 23 (23 - 0) dBm as advertised by
> 54:ec:2f:05:70:2c

Is that the message you meant?

> Another _almost_ successful suspend/resume:
> [snip
>    wlp2s0: RX AssocResp from 54:ec:2f:05:70:2c (capab=0x1011 status=0 aid=2)
>    wlp2s0: associated
>    wlp2s0: Limiting TX power to 23 (23 - 0) dBm as advertised by
> 54:ec:2f:05:70:2c
>    ath: EEPROM regdomain: 0x826c
>    ath: EEPROM indicates we should expect a country code
>    ath: doing EEPROM country->regdmn map search
>    ath: country maps to regdmn code: 0x37
>    ath: Country alpha2 being used: PT
>    ath: Regpair used: 0x37
>    ath: regdomain 0x826c dynamically updated by country element
>    IPv6: ADDRCONF(NETDEV_CHANGE): wlp2s0: link becomes ready
> 
> I say _almost_, because I don't see the "No TX power limit" for the
> country lookup (yes, Portugal) this time?

because here you had it too, just a bit earlier. It usually comes when a
beacon is received the first time, which depends on the AP timing.

>    ath10k_pci 0000:02:00.0: wmi command 16387 timeout, restarting hardware
>    ath10k_pci 0000:02:00.0: failed to set 5g txpower 23: -11
>    ath10k_pci 0000:02:00.0: failed to setup tx power 23: -11
>    ath10k_pci 0000:02:00.0: failed to recalc tx power: -11
>    ath10k_pci 0000:02:00.0: failed to set inactivity time for vdev 0: -108
>    ath10k_pci 0000:02:00.0: failed to setup powersave: -108
> 
> That certainly looks like something did try to set a power limit, but
> eventually failed.

Yeah, that does seem a bit fishy. Kalle would have to comment for
ath10k.

> Immediately after that:
> 
>    wlp2s0: deauthenticating from 54:ec:2f:05:70:2c by local choice
> (Reason: 3=DEAUTH_LEAVING)

I don't _think_ any of the above would be a reason to disconnect, but it
clearly looks like the device got stuck at this point, since everything
just fails afterwards.

>    ath10k_pci 0000:02:00.0: failed to delete peer 54:ec:2f:05:70:2c
> for vdev 0: -108
> 
> and this then results in:
> 
>    WARNING: CPU: 4 PID: 1246 at net/mac80211/sta_info.c:1057
> __sta_info_destroy_part2+0x147/0x150 [mac80211]

Not really a surprise. Perhaps we shouldn't even WARN_ON() this, if the
driver is stuck completely and returning errors to everything that
doesn't help so much.

Then again, the stack trace was helpful this time:

>     ieee80211_set_disassoc+0xc2/0x590 [mac80211]
>     ieee80211_mgd_deauth.cold+0x4a/0x1b8 [mac80211]
>     cfg80211_mlme_deauth+0xb3/0x1d0 [cfg80211]
>     cfg80211_mlme_down+0x66/0x90 [cfg80211]
>     cfg80211_disconnect+0x129/0x1e0 [cfg80211]
>     cfg80211_leave+0x27/0x40 [cfg80211]
>     cfg80211_netdev_notifier_call+0x1a7/0x4e0 [cfg80211]
>     notifier_call_chain+0x4c/0x70
>     __dev_close_many+0x57/0x100
>     dev_close_many+0x8d/0x140
>     dev_close.part.0+0x44/0x70
>     cfg80211_shutdown_all_interfaces+0x71/0xd0 [cfg80211]
>     cfg80211_rfkill_set_block+0x22/0x30 [cfg80211]
>     rfkill_set_block+0x92/0x140 [rfkill]
>     rfkill_fop_write+0x11f/0x1c0 [rfkill]
>     vfs_write+0xb6/0x1a0


Since we see that something actually did an rfkill operation. Did you
push a button there?

Like I said above, the fact that we get into notifier_call_chain() from
rfkill_set_block() means that we acquired the RTNL here somewhere
between these.

>    ath10k_pci 0000:02:00.0: failed to recalculate rts/cts prot for vdev 0: -108
>    ath10k_pci 0000:02:00.0: failed to set cts protection for vdev 0: -108
> 
> and it looks like it leaves some lock held.

Yeah, the RTNL.

You don't happen to have timing information on these logs, perhaps
recorded in the logfile/journal?

It seems odd to me, since the RTNL is acquired by
cfg80211_rfkill_set_block() and that doesn't even have an error path, it
just does
        rtnl_lock();
        cfg80211_shutdown_all_interfaces(&rdev->wiphy);
        rtnl_unlock();

The only explanation I therefore have is that something is just taking
*forever* in that code path, hence my question about timing information
on the logs.

Looks like indeed the driver gives the device at least *3 seconds* for
every command, see ath10k_wmi_cmd_send(), so most likely this would
eventually have finished, but who knows how many firmware commands it
would still have attempted to send...

Perhaps the driver should mark the device as dead and fail quickly once
it timed out once, or so, but I'll let Kalle comment on that.

johannes


^ permalink raw reply

* [PATCH] NFC: st95hf: fix spelling mistake "receieve" -> "receive"
From: Colin King @ 2019-09-11 10:38 UTC (permalink / raw)
  To: David S . Miller, Greg Kroah-Hartman, netdev
  Cc: kernel-janitors, linux-kernel

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

There is a spelling mistake in a dev_err message. Fix it.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/nfc/st95hf/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 6034c5bf49d9..ce38782ebf80 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -316,7 +316,7 @@ static int st95hf_echo_command(struct st95hf_context *st95context)
 					  &echo_response);
 	if (result) {
 		dev_err(&st95context->spicontext.spidev->dev,
-			"err: echo response receieve error = 0x%x\n", result);
+			"err: echo response receive error = 0x%x\n", result);
 		return result;
 	}
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] bpf: validate bpf_func when BPF_JIT is enabled
From: Björn Töpel @ 2019-09-11 10:39 UTC (permalink / raw)
  To: Yonghong Song, Sami Tolvanen
  Cc: Alexei Starovoitov, Daniel Borkmann, Kees Cook, Martin Lau,
	Song Liu, netdev@vger.kernel.org, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer
In-Reply-To: <c7c7668e-6336-0367-42b3-2f6026c466dd@fb.com>


On 2019-09-11 09:42, Yonghong Song wrote:
> I am not an expert in XDP testing. Toke, Björn, could you give some
> suggestions what to test for XDP performance here?

I ran the "xdp_rxq_info" sample with and without Sami's patch:

$ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP

Before:

Running XDP on dev:enp134s0f0 (ifindex:6) action:XDP_DROP options:no_touch
XDP stats       CPU     pps         issue-pps
XDP-RX CPU      20      23923874    0
XDP-RX CPU      total   23923874

RXQ stats       RXQ:CPU pps         issue-pps
rx_queue_index   20:20  23923878    0
rx_queue_index   20:sum 23923878

After Sami's patch:

Running XDP on dev:enp134s0f0 (ifindex:6) action:XDP_DROP options:no_touch
XDP stats       CPU     pps         issue-pps
XDP-RX CPU      20      22998700    0
XDP-RX CPU      total   22998700

RXQ stats       RXQ:CPU pps         issue-pps
rx_queue_index   20:20  22998705    0
rx_queue_index   20:sum 22998705


So, roughly ~4% for this somewhat naive scenario.


As for XDP performance tests; I guess some of the XDP selftests could be
used as well!


Björn

^ permalink raw reply

* Re: [PATCH bpf-next 01/11] samples: bpf: makefile: fix HDR_PROBE "echo"
From: Sergei Shtylyov @ 2019-09-11 11:02 UTC (permalink / raw)
  To: ast, daniel, yhs, davem, jakub.kicinski, hawk, john.fastabend,
	linux-kernel, netdev, bpf, clang-built-linux
In-Reply-To: <20190910145359.GD3053@khorivan>

On 10.09.2019 17:54, Ivan Khoronzhuk wrote:

>> Hello!
>>
>> On 10.09.2019 13:38, Ivan Khoronzhuk wrote:
>>
>>> echo should be replaced on echo -e to handle \n correctly, but instead,
>>
>>  s/on/with/?
> s/echo/printf/ instead of s/echo/echo -e/

    I only pointed that 'on' is incorrect there. You replace something /with/ 
something other...

> 
> printf looks better.
> 
>>
>>> replace it on printf as some systems can't handle echo -e.
>>
>>   Likewise?

    Same grammatical mistake.

> I can guess its Mac vs Linux, but it does mean nothing if it's defined as
> implementation dependent, can be any.
 >
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> [...]

MBR, Sergei

^ permalink raw reply

* [PATCH net-next 0/2] devlink: add unknown 'fw_load_policy' value
From: Simon Horman @ 2019-09-11 11:08 UTC (permalink / raw)
  To: David Miller
  Cc: Jakub Kicinski, netdev, oss-drivers, Dirk van der Merwe,
	Simon Horman

Dirk says:

Recently we added an unknown value for the 'reset_dev_on_drv_probe' devlink
parameter. Extend the 'fw_load_policy' parameter in the same way.

The only driver that uses this right now is the nfp driver.

Dirk van der Merwe (2):
  devlink: add unknown 'fw_load_policy' value
  nfp: devlink: set unknown fw_load_policy

 drivers/net/ethernet/netronome/nfp/devlink_param.c | 3 ++-
 include/uapi/linux/devlink.h                       | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.11.0


^ 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