Netdev List
 help / color / mirror / Atom feed
* [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: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

* 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 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: [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

* [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: [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

* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Xin Long @ 2019-09-11  9:21 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: <1e5c3163e6c649b09137eeb62d193d87@AcuMS.aculab.com>

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?

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

^ permalink raw reply

* Re: [PATCH 1/7] net/dsa: configure autoneg for CPU port
From: Robert Beckett @ 2019-09-11  9:16 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn; +Cc: netdev, Vivien Didelot, David S. Miller
In-Reply-To: <aa0459e0-64ee-de84-fc38-3c9364301275@gmail.com>

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.



^ permalink raw reply

* RE: [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
From: Jose Abreu @ 2019-09-11  9:15 UTC (permalink / raw)
  To: Florian Fainelli, Jose Abreu, Thierry Reding
  Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
	Jon Hunter, Bitan Biswas, netdev@vger.kernel.org,
	linux-tegra@vger.kernel.org
In-Reply-To: <c8d419d3-6cf6-e260-a2e2-6a339c6c321b@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sep/10/2019, 20:01:01 (UTC+00:00)

> On 9/10/19 1:35 AM, Jose Abreu wrote:
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Date: Sep/09/2019, 20:13:29 (UTC+00:00)
> > 
> >> On Mon, Sep 09, 2019 at 04:05:52PM +0000, Jose Abreu wrote:
> >>> From: Thierry Reding <thierry.reding@gmail.com>
> >>> Date: Sep/09/2019, 16:25:46 (UTC+00:00)
> >>>
> >>>> @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
> >>>>  	value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
> >>>>  	writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
> >>>>  
> >>>> +	if (dma_cfg->eame)
> >>>
> >>> There is no need for this check. If EAME is not enabled then upper 32 
> >>> bits will be zero.
> >>
> >> The idea here was to potentially guard against this register not being
> >> available on some revisions. Having the check here would avoid access to
> >> the register if the device doesn't support enhanced addressing.
> > 
> > I see your point but I don't think there will be any problems unless you 
> > have some strange system that doesn't handle the write accesses to 
> > unimplemented features properly ...
> 
> Is not it then just safer to not do the write to a register that you do
> not know how the implementation is going to respond to with one of a
> target abort, timeout, decoding error, just dead lock?

I don't think any of these will ever happen. Notice that this is already 
been done for a long time in some registers that may not exist in some 
random HW config and there is also the point that this is a write 
operation so Slave Error would only get triggered if we did a read.

> Also, would it make sense to consider adding an #ifdef
> CONFIG_PHYS_ADDR_T_64BIT plus the conditional check so that you can be
> slightly more optimal in the hot-path here?

Well, this is not hot-path. It's only done in HW open sequence. The 
hot-path would be set_{rx/tx}_tail_ptr() but that's 32 bits only. 

---
Thanks,
Jose Miguel Abreu

^ permalink raw reply

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

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.

	David

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

^ permalink raw reply

* Re: linux-next: Signed-off-by missing for commit in the net-next tree
From: Oleksij Rempel @ 2019-09-11  9:02 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, Networking, Linux Next Mailing List,
	Linux Kernel Mailing List, The j1939 authors, Bastian Stender,
	Elenita Hinds, Kurt Van Dijck, kbuild test robot, Maxime Jayat,
	Robin van der Gracht, Marc Kleine-Budde
In-Reply-To: <20190911004103.3480fa40@canb.auug.org.au>

Hi Stephen,

On Wed, Sep 11, 2019 at 12:41:03AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   9d71dd0c7009 ("can: add support of SAE J1939 protocol")
> 
> is missing a Signed-off-by from its author.
> 
> [Not sure if I should complain about this one ...]

Here is the original pull request message for this patch series:
"The final patch is the collective effort of many entities (The j1939
authors: Oliver Hartkopp, Bastian Stender, Elenita Hinds, kbuild test
robot, Kurt Van Dijck, Maxime Jayat, Robin van der Gracht, Oleksij
Rempel, Marc Kleine-Budde). It adds support of SAE J1939 protocol to the
CAN networking stack."
https://www.mail-archive.com/netdev@vger.kernel.org/msg313476.html

Since the patch can be hardly assigned to one author we deiced to use
address of CAN mailing list.

Best regards,
Oleksij Rempel
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* RE: [PATCH net-next 0/6] net: stmmac: Improvements for -next
From: Jose Abreu @ 2019-09-11  8:59 UTC (permalink / raw)
  To: David Miller, Jose.Abreu@synopsys.com
  Cc: netdev@vger.kernel.org, Joao.Pinto@synopsys.com,
	peppe.cavallaro@st.com, alexandre.torgue@st.com,
	mcoquelin.stm32@gmail.com,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190911.102155.148817974369878410.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sep/11/2019, 09:21:55 (UTC+00:00)

> From: Jose Abreu <Jose.Abreu@synopsys.com>
> Date: Tue, 10 Sep 2019 16:41:21 +0200
> 
> > Misc patches for -next. It includes:
> >  - Two fixes for features in -next only
> >  - New features support for GMAC cores (which includes GMAC4 and GMAC5)
> 
> Series applied, but what exactly does "ARP offload" even do?

ARP Offload allows the IP to reply to ARP_REQUEST packets automatically 
without passing by the application.

As net doesn't support this offloading I'm currently using this feature 
to test endianness issues in the IP and check if MAC Address is 
correctly configured, the logic is as follows:
 - MAC is set in loopback mode and ARP offload is activated
 - selftests create a dummy ARP_REQUEST packet and send it out
 - IP will detect the ARP_REQUEST packet and generate an ARP_REPLY 
packet
 - As MAC is in loopback mode then selftests will receive the ARP_REPLY 
packet
 - selftests logic will check if ARP_REPLY packet is correct (i.e. MAC 
address and packet type)

This way if this test fails it probably indicates that MAC address of IP 
is not correctly configured or that endianness of the IP was changed 
from default setting (which is LE).

By default the feature is off because user may not want to reply to 
ARP_REQUEST and I'm more using it as a diagnose facility. Let me know if 
you agree with this approach.

---
Thanks,
Jose Miguel Abreu

^ permalink raw reply

* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Xin Long @ 2019-09-11  8:51 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: <9fc7ca1598e641cda3914840a4416aab@AcuMS.aculab.com>

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;
        }

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

in sctp_getsockopt_paddr_thresholds():

        if (len >= sizeof(struct sctp_paddrthlds))
                len = sizeof(struct sctp_paddrthlds);
        else if (len >= ALIGN(offsetof(struct sctp_paddrthlds,
                                       spt_pathcpthld), 4))
                len = ALIGN(offsetof(struct sctp_paddrthlds,
                                     spt_pathcpthld), 4);
        else
                return -EINVAL;

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

>
> So adding an extra field breaks existing application binaries
> that use this option.
>
> I've not checked the other patches or similar fubar.
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

^ permalink raw reply

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
From: Dan Carpenter @ 2019-09-11  8:30 UTC (permalink / raw)
  To: maowenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors
In-Reply-To: <53556c87-a351-4314-cbd9-49a39d0b41aa@huawei.com>

On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> 
> 
> On 2019/9/11 3:22, Dan Carpenter wrote:
> > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> >>> There are more parentheses in if clause when call sctp_get_port_local
> >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> >>> do cleanup.
> >>>
> >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >>> ---
> >>>  net/sctp/socket.c | 3 +--
> >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> >>> index 9d1f83b10c0a..766b68b55ebe 100644
> >>> --- a/net/sctp/socket.c
> >>> +++ b/net/sctp/socket.c
> >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> >>>  	 * detection.
> >>>  	 */
> >>>  	addr->v4.sin_port = htons(snum);
> >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> >>> +	if (sctp_get_port_local(sk, addr))
> >>>  		return -EADDRINUSE;
> >>
> >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> >> casted to long.  It's not documented what it means and neither of the
> >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> >> sctp_get_port").
> > 
> > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > because before the code assumed that a pointer casted to an int was the
> > same as a pointer casted to a long.
> 
> commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> cleanup is ok?

Yeah.  It's fine, I was just confused why we weren't preserving the
error code and then I saw that we didn't return errors at all and got
confused.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH] lib/Kconfig: fix OBJAGG in lib/ menu structure
From: David Miller @ 2019-09-11  8:30 UTC (permalink / raw)
  To: rdunlap; +Cc: netdev, linux-kernel, jiri, idosch
In-Reply-To: <34674398-54dc-a4d1-6052-67ad1a3b2fe9@infradead.org>

From: Randy Dunlap <rdunlap@infradead.org>
Date: Mon, 9 Sep 2019 14:54:21 -0700

> From: Randy Dunlap <rdunlap@infradead.org>
> 
> Keep the "Library routines" menu intact by moving OBJAGG into it.
> Otherwise OBJAGG is displayed/presented as an orphan in the
> various config menus.
> 
> Fixes: 0a020d416d0a ("lib: introduce initial implementation of object aggregation manager")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>

Applied, thanks Randy.

^ permalink raw reply

* Re: [pull request][net-next 0/3] Mellanox, mlx5 build cleanup 2019-09-10
From: David Miller @ 2019-09-11  8:27 UTC (permalink / raw)
  To: saeedm; +Cc: netdev
In-Reply-To: <20190910214542.8433-1-saeedm@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Tue, 10 Sep 2019 21:45:57 +0000

> This series provides three build warnings cleanup patches for mlx5,
> Originally i wanted to wait a bit more and attach more patches to this
> series, but apparently this can't wait since already 3 different patches
> for the same fix were submitted this week :).
> 
> For more information please see tag log below.
> 
> Please pull and let me know if there is any problem.

Pulled, thanks.

^ permalink raw reply

* Re: [net-next 11/14] ixgbe: Prevent u8 wrapping of ITR value to something less than 10us
From: David Miller @ 2019-09-11  8:24 UTC (permalink / raw)
  To: jeffrey.t.kirsher
  Cc: alexander.h.duyck, netdev, nhorman, sassmann, gleventhal,
	andrewx.bowers
In-Reply-To: <20190910163434.2449-12-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Sep 2019 09:34:31 -0700

> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> There were a couple cases where the ITR value generated via the adaptive
> ITR scheme could exceed 126. This resulted in the value becoming either 0
> or something less than 10. Switching back and forth between a value less
> than 10 and a value greater than 10 can cause issues as certain hardware
> features such as RSC to not function well when the ITR value has dropped
> that low.
> 
> Reported-by: Gregg Leventhal <gleventhal@janestreet.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Please remove this from the series, add the Fixes: tag, and submit for 'net'
since Alexander says it is -stable material.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/6] net: stmmac: Improvements for -next
From: David Miller @ 2019-09-11  8:21 UTC (permalink / raw)
  To: Jose.Abreu
  Cc: netdev, Joao.Pinto, peppe.cavallaro, alexandre.torgue,
	mcoquelin.stm32, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1568126224.git.joabreu@synopsys.com>

From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Tue, 10 Sep 2019 16:41:21 +0200

> Misc patches for -next. It includes:
>  - Two fixes for features in -next only
>  - New features support for GMAC cores (which includes GMAC4 and GMAC5)

Series applied, but what exactly does "ARP offload" even do?

^ permalink raw reply

* Re: [PATCH v7 6/7] nfc: pn533: Add autopoll capability
From: David Miller @ 2019-09-11  8:17 UTC (permalink / raw)
  To: poeschel
  Cc: allison, keescook, opensource, swinslow, gregkh, gustavo, tglx,
	kstewart, netdev, linux-kernel, johan
In-Reply-To: <20190910093415.2186-1-poeschel@lemonage.de>

From: Lars Poeschel <poeschel@lemonage.de>
Date: Tue, 10 Sep 2019 11:34:12 +0200

> +static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
> +			       struct sk_buff *resp)
> +{
> +	u8 nbtg;
> +	int rc;
> +	struct pn532_autopoll_resp *apr;
> +	struct nfc_target nfc_tgt;

Need reverse christmas tree here.

> @@ -1534,6 +1655,7 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
>  	struct pn533_poll_modulations *cur_mod;
>  	u8 rand_mod;
>  	int rc;
> +	struct sk_buff *skb;

Likewise.

^ permalink raw reply

* Re: [PATCH bpf-next v10 2/4] bpf: new helper to obtain namespace data from current task New bpf helper bpf_get_current_pidns_info.
From: Eric W. Biederman @ 2019-09-11  8:17 UTC (permalink / raw)
  To: Carlos Antonio Neira Bustos
  Cc: Yonghong Song, Al Viro, netdev@vger.kernel.org, brouer@redhat.com,
	bpf@vger.kernel.org
In-Reply-To: <20190911043225.GA22183@frodo.byteswizards.com>

Carlos Antonio Neira Bustos <cneirabustos@gmail.com> writes:

> On Tue, Sep 10, 2019 at 10:35:09PM +0000, Yonghong Song wrote:
> Thanks a lot Yonghong.
> I'll include this patch when submitting changes for version 11 of
> this patch.

Please see my reply to Al.

Eric



^ permalink raw reply

* Re: [PATCH v7 5/7] nfc: pn533: add UART phy driver
From: David Miller @ 2019-09-11  8:17 UTC (permalink / raw)
  To: poeschel
  Cc: gregkh, tglx, kstewart, swinslow, allison, linux-kernel, netdev,
	johan, Claudiu.Beznea
In-Reply-To: <20190910093359.2110-1-poeschel@lemonage.de>

From: Lars Poeschel <poeschel@lemonage.de>
Date: Tue, 10 Sep 2019 11:33:50 +0200

> +static int pn532_uart_send_ack(struct pn533 *dev, gfp_t flags)
> +{
> +	struct pn532_uart_phy *pn532 = dev->phy;
> +	/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
> +	static const u8 ack[PN533_STD_FRAME_ACK_SIZE] = {
> +			0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
> +	int err;

Reverse christmas tree ordering for the local variables please.

> +static int pn532_uart_rx_is_frame(struct sk_buff *skb)
> +{
> +	int i;
> +	u16 frame_len;
> +	struct pn533_std_frame *std;
> +	struct pn533_ext_frame *ext;

Likewise.

^ permalink raw reply

* Re: [PATCH bpf-next v10 2/4] bpf: new helper to obtain namespace data from current task New bpf helper bpf_get_current_pidns_info.
From: Eric W. Biederman @ 2019-09-11  8:16 UTC (permalink / raw)
  To: Al Viro
  Cc: Yonghong Song, Carlos Antonio Neira Bustos,
	netdev@vger.kernel.org, brouer@redhat.com, bpf@vger.kernel.org
In-Reply-To: <20190910231506.GL1131@ZenIV.linux.org.uk>

Al Viro <viro@zeniv.linux.org.uk> writes:

> On Tue, Sep 10, 2019 at 10:35:09PM +0000, Yonghong Song wrote:
>> 
>> Carlos,
>> 
>> Discussed with Eric today for what is the best way to get
>> the device number for a namespace. The following patch seems
>> a reasonable start although Eric would like to see
>> how the helper is used in order to decide whether the
>> interface looks right.
>> 
>> commit bb00fc36d5d263047a8bceb3e51e969d7fbce7db (HEAD -> fs2)
>> Author: Yonghong Song <yhs@fb.com>
>> Date:   Mon Sep 9 21:50:51 2019 -0700
>> 
>>      nsfs: add an interface function ns_get_inum_dev()
>> 
>>      This patch added an interface function
>>      ns_get_inum_dev(). Given a ns_common structure,
>>      the function returns the inode and device
>>      numbers. The function will be used later
>>      by a newly added bpf helper.
>> 
>>      Signed-off-by: Yonghong Song <yhs@fb.com>
>> 
>> diff --git a/fs/nsfs.c b/fs/nsfs.c
>> index a0431642c6b5..a603c6fc3f54 100644
>> --- a/fs/nsfs.c
>> +++ b/fs/nsfs.c
>> @@ -245,6 +245,14 @@ struct file *proc_ns_fget(int fd)
>>          return ERR_PTR(-EINVAL);
>>   }
>> 
>> +/* Get the device number for the current task pidns.
>> + */
>> +void ns_get_inum_dev(struct ns_common *ns, u32 *inum, dev_t *dev)
>> +{
>> +       *inum = ns->inum;
>> +       *dev = nsfs_mnt->mnt_sb->s_dev;
>> +}
>
> Umm...  Where would it get the device number once we get (hell knows
> what for) multiple nsfs instances?  I still don't understand what
> would that be about, TBH...  Is it really per-userns?  Or something
> else entirely?  Eric, could you give some context?

My goal is not to paint things into a corner, with future changes.
Right now it is possible to stat a namespace file descriptor and
get a device and inode number.  Then compare that. 

I don't want people using the inode number in nsfd as some magic
namespace id.

We have had times in the past where there was more than one superblock
and thus more than one device number.  Further if userspace ever uses
this heavily there may be times in the future where for
checkpoint/restart purposes we will want multiple nsfd's so we can
preserve the inode number accross a migration.

Realistically there will probably just some kind of hotplug notification
to userspace to say we have hotplugged your operatining system as
a migration notification.

Now the halway discussion did not quite capture everything I was trying
to say but it at least got to the right ballpark.

The helper in fs/nsfs.c should be:

bool ns_match(const struct ns_common *ns, dev_t dev, ino_t ino)
{
        return ((ns->inum == ino) && (nsfs_mnt->mnt_sb->s_dev == dev));
}

That way if/when there are multiple inodes identifying the same
namespace the bpf programs don't need to change.

Up farther in the stack it should be something like:

> BPF_CALL_2(bpf_current_pidns_match, dev_t *dev, ino_t *ino)
> {
>         return ns_match(&task_active_pid_ns(current)->ns, *dev, *ino);
> }
> 
> const struct bpf_func_proto bpf_current_pidns_match_proto = {
> 	.func		= bpf_current_pins_match,
> 	.gpl_only	= true,
> 	.ret_type	= RET_INTEGER
> 	.arg1_type	= ARG_PTR_TO_DEVICE_NUMBER,
> 	.arg2_type	= ARG_PTR_TO_INODE_NUMBER,
> };

That allows comparing what the bpf came up with with whatever value
userspace generated by stating the file descriptor.


That is the least bad suggestion I currently have for that
functionality.  It really would be better to not have that filter in the
bpf program itself but in the infrastructure that binds a program to a
set of tasks.

The problem with this approach is whatever device/inode you have when
the namespace they refer to exits there is the possibility that the
inode will be reused.  So your filter will eventually start matching on
the wrong thing.

Eric

^ permalink raw reply

* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Xin Long @ 2019-09-11  8:14 UTC (permalink / raw)
  To: David Miller
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, Neil Horman
In-Reply-To: <20190910.192755.717621354475214603.davem@davemloft.net>

On Wed, Sep 11, 2019 at 1:27 AM David Miller <davem@davemloft.net> wrote:
>
> From: Xin Long <lucien.xin@gmail.com>
> Date: Mon,  9 Sep 2019 15:56:51 +0800
>
> > 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;
> >  };
> >
> >  /*
>
> As pointed out you can't add things to this structure without breaking existing
> binaries.
we had the same problem when adding:
spp_ipv6_flowlabel and spp_dscp into struct sctp_paddrparams. in:

commit 0b0dce7a36fb9f1a9dd8245ea82d3a268c6943fe
Author: Xin Long <lucien.xin@gmail.com>
Date:   Mon Jul 2 18:21:13 2018 +0800

    sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

the solution was:

        if (optlen == sizeof(params)) {
                if (copy_from_user(&params, optval, optlen))
                        return -EFAULT;
        } else if (optlen == ALIGN(offsetof(struct sctp_paddrparams,
                                            spp_ipv6_flowlabel), 4)) {
                if (copy_from_user(&params, optval, optlen))
                        return -EFAULT;
                if (params.spp_flags & (SPP_DSCP | SPP_IPV6_FLOWLABEL))
                        return -EINVAL;
        } else {
                return -EINVAL;
        }

I will do the same for this patch. Thanks.

^ permalink raw reply

* Re: [PATCH v2 net] net: sonic: replace dev_kfree_skb in sonic_send_packet
From: David Miller @ 2019-09-11  8:14 UTC (permalink / raw)
  To: maowenan; +Cc: tsbogend, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20190911013623.36897-1-maowenan@huawei.com>

From: Mao Wenan <maowenan@huawei.com>
Date: Wed, 11 Sep 2019 09:36:23 +0800

> sonic_send_packet will be processed in irq or non-irq 
> context, so it would better use dev_kfree_skb_any
> instead of dev_kfree_skb.
> 
> Fixes: d9fb9f384292 ("*sonic/natsemi/ns83829: Move the National Semi-conductor drivers")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>

Applied.

^ 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