* Re: [PATCH] net/phy: fix DP83865 10 Mbps HDX loopback disable function
From: Andrew Lunn @ 2019-09-18 14:19 UTC (permalink / raw)
To: Peter Mamonov; +Cc: Florian Fainelli, netdev, linux-kernel
In-Reply-To: <20190918140340.21032-1-pmamonov@gmail.com>
On Wed, Sep 18, 2019 at 05:03:40PM +0300, Peter Mamonov wrote:
> According to the DP83865 datasheet "The 10 Mbps HDX loopback can be
> disabled in the expanded memory register 0x1C0.1." The driver erroneously
> used bit 0 instead of bit 1.
>
> Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
> ---
> drivers/net/phy/national.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
> index 2addf1d3f619..4892e785dbf3 100644
> --- a/drivers/net/phy/national.c
> +++ b/drivers/net/phy/national.c
> @@ -110,11 +110,14 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
>
> static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable)
> {
> + u16 lb_dis = 1 << 1;
Hi Peter
Please use the BIT() macro.
> +
> if (disable)
> - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1);
> + ns_exp_write(phydev, 0x1c0,
> + ns_exp_read(phydev, 0x1c0) | lb_dis);
> else
> ns_exp_write(phydev, 0x1c0,
> - ns_exp_read(phydev, 0x1c0) & 0xfffe);
> + ns_exp_read(phydev, 0x1c0) & ~lb_dis);
>
> pr_debug("10BASE-T HDX loopback %s\n",
> (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on");
Isn't this also wrong?
Andrew
^ permalink raw reply
* Re: [RFC v4 0/3] vhost: introduce mdev based hardware backend
From: Michael S. Tsirkin @ 2019-09-18 14:32 UTC (permalink / raw)
To: Jason Wang
Cc: Tiwei Bie, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <fa6957f3-19ad-f351-8c43-65bc8342b82e@redhat.com>
On Wed, Sep 18, 2019 at 01:51:21PM +0800, Jason Wang wrote:
>
> On 2019/9/17 下午6:58, Tiwei Bie wrote:
> > On Tue, Sep 17, 2019 at 11:32:03AM +0800, Jason Wang wrote:
> > > On 2019/9/17 上午9:02, Tiwei Bie wrote:
> > > > This RFC is to demonstrate below ideas,
> > > >
> > > > a) Build vhost-mdev on top of the same abstraction defined in
> > > > the virtio-mdev series [1];
> > > >
> > > > b) Introduce /dev/vhost-mdev to do vhost ioctls and support
> > > > setting mdev device as backend;
> > > >
> > > > Now the userspace API looks like this:
> > > >
> > > > - Userspace generates a compatible mdev device;
> > > >
> > > > - Userspace opens this mdev device with VFIO API (including
> > > > doing IOMMU programming for this mdev device with VFIO's
> > > > container/group based interface);
> > > >
> > > > - Userspace opens /dev/vhost-mdev and gets vhost fd;
> > > >
> > > > - Userspace uses vhost ioctls to setup vhost (userspace should
> > > > do VHOST_MDEV_SET_BACKEND ioctl with VFIO group fd and device
> > > > fd first before doing other vhost ioctls);
> > > >
> > > > Only compile test has been done for this series for now.
> > >
> > > Have a hard thought on the architecture:
> > Thanks a lot! Do appreciate it!
> >
> > > 1) Create a vhost char device and pass vfio mdev device fd to it as a
> > > backend and translate vhost-mdev ioctl to virtio mdev transport (e.g
> > > read/write). DMA was done through the VFIO DMA mapping on the container that
> > > is attached.
> > Yeah, that's what we are doing in this series.
> >
> > > We have two more choices:
> > >
> > > 2) Use vfio-mdev but do not create vhost-mdev device, instead, just
> > > implement vhost ioctl on vfio_device_ops, and translate them into
> > > virtio-mdev transport or just pass ioctl to parent.
> > Yeah. Instead of introducing /dev/vhost-mdev char device, do
> > vhost ioctls on VFIO device fd directly. That's what we did
> > in RFC v3.
> >
> > > 3) Don't use vfio-mdev, create a new vhost-mdev driver, during probe still
> > > try to add dev to vfio group and talk to parent with device specific ops
> > If my understanding is correct, this means we need to introduce
> > a new VFIO device driver to replace the existing vfio-mdev driver
> > in our case. Below is a quick draft just to show my understanding:
> >
> > #include <linux/init.h>
> > #include <linux/module.h>
> > #include <linux/device.h>
> > #include <linux/kernel.h>
> > #include <linux/slab.h>
> > #include <linux/vfio.h>
> > #include <linux/mdev.h>
> >
> > #include "mdev_private.h"
> >
> > /* XXX: we need a proper way to include below vhost header. */
> > #include "../../vhost/vhost.h"
> >
> > static int vfio_vhost_mdev_open(void *device_data)
> > {
> > if (!try_module_get(THIS_MODULE))
> > return -ENODEV;
> >
> > /* ... */
> > vhost_dev_init(...);
> >
> > return 0;
> > }
> >
> > static void vfio_vhost_mdev_release(void *device_data)
> > {
> > /* ... */
> > module_put(THIS_MODULE);
> > }
> >
> > static long vfio_vhost_mdev_unlocked_ioctl(void *device_data,
> > unsigned int cmd, unsigned long arg)
> > {
> > struct mdev_device *mdev = device_data;
> > struct mdev_parent *parent = mdev->parent;
> >
> > /*
> > * Use vhost ioctls.
> > *
> > * We will have a different parent_ops design.
> > * And potentially, we can share the same parent_ops
> > * with virtio_mdev.
> > */
> > switch (cmd) {
> > case VHOST_GET_FEATURES:
> > parent->ops->get_features(mdev, ...);
> > break;
> > /* ... */
> > }
> >
> > return 0;
> > }
> >
> > static ssize_t vfio_vhost_mdev_read(void *device_data, char __user *buf,
> > size_t count, loff_t *ppos)
> > {
> > /* ... */
> > return 0;
> > }
> >
> > static ssize_t vfio_vhost_mdev_write(void *device_data, const char __user *buf,
> > size_t count, loff_t *ppos)
> > {
> > /* ... */
> > return 0;
> > }
> >
> > static int vfio_vhost_mdev_mmap(void *device_data, struct vm_area_struct *vma)
> > {
> > /* ... */
> > return 0;
> > }
> >
> > static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = {
> > .name = "vfio-vhost-mdev",
> > .open = vfio_vhost_mdev_open,
> > .release = vfio_vhost_mdev_release,
> > .ioctl = vfio_vhost_mdev_unlocked_ioctl,
> > .read = vfio_vhost_mdev_read,
> > .write = vfio_vhost_mdev_write,
> > .mmap = vfio_vhost_mdev_mmap,
> > };
> >
> > static int vfio_vhost_mdev_probe(struct device *dev)
> > {
> > struct mdev_device *mdev = to_mdev_device(dev);
> >
> > /* ... */
> > return vfio_add_group_dev(dev, &vfio_vhost_mdev_dev_ops, mdev);
> > }
> >
> > static void vfio_vhost_mdev_remove(struct device *dev)
> > {
> > /* ... */
> > vfio_del_group_dev(dev);
> > }
> >
> > static struct mdev_driver vfio_vhost_mdev_driver = {
> > .name = "vfio_vhost_mdev",
> > .probe = vfio_vhost_mdev_probe,
> > .remove = vfio_vhost_mdev_remove,
> > };
> >
> > static int __init vfio_vhost_mdev_init(void)
> > {
> > return mdev_register_driver(&vfio_vhost_mdev_driver, THIS_MODULE);
> > }
> > module_init(vfio_vhost_mdev_init)
> >
> > static void __exit vfio_vhost_mdev_exit(void)
> > {
> > mdev_unregister_driver(&vfio_vhost_mdev_driver);
> > }
> > module_exit(vfio_vhost_mdev_exit)
>
>
> Yes, something like this basically.
>
>
> > > So I have some questions:
> > >
> > > 1) Compared to method 2, what's the advantage of creating a new vhost char
> > > device? I guess it's for keep the API compatibility?
> > One benefit is that we can avoid doing vhost ioctls on
> > VFIO device fd.
>
>
> Yes, but any benefit from doing this?
It does seem a bit more modular, but it's certainly not a big deal.
> >
> > > 2) For method 2, is there any easy way for user/admin to distinguish e.g
> > > ordinary vfio-mdev for vhost from ordinary vfio-mdev?
> > I think device-api could be a choice.
>
>
> Ok.
>
>
> >
> > > I saw you introduce
> > > ops matching helper but it's not friendly to management.
> > The ops matching helper is just to check whether a given
> > vfio-device is based on a mdev device.
> >
> > > 3) A drawback of 1) and 2) is that it must follow vfio_device_ops that
> > > assumes the parameter comes from userspace, it prevents support kernel
> > > virtio drivers.
> > >
> > > 4) So comes the idea of method 3, since it register a new vhost-mdev driver,
> > > we can use device specific ops instead of VFIO ones, then we can have a
> > > common API between vDPA parent and vhost-mdev/virtio-mdev drivers.
> > As the above draft shows, this requires introducing a new
> > VFIO device driver. I think Alex's opinion matters here.
>
>
> Yes, it is.
>
> Thanks
>
>
> > Thanks,
> > Tiwei
> >
> > > What's your thoughts?
> > >
> > > Thanks
> > >
> > >
> > > > RFCv3: https://patchwork.kernel.org/patch/11117785/
> > > >
> > > > [1] https://lkml.org/lkml/2019/9/10/135
> > > >
> > > > Tiwei Bie (3):
> > > > vfio: support getting vfio device from device fd
> > > > vfio: support checking vfio driver by device ops
> > > > vhost: introduce mdev based hardware backend
> > > >
> > > > drivers/vfio/mdev/vfio_mdev.c | 3 +-
> > > > drivers/vfio/vfio.c | 32 +++
> > > > drivers/vhost/Kconfig | 9 +
> > > > drivers/vhost/Makefile | 3 +
> > > > drivers/vhost/mdev.c | 462 +++++++++++++++++++++++++++++++
> > > > drivers/vhost/vhost.c | 39 ++-
> > > > drivers/vhost/vhost.h | 6 +
> > > > include/linux/vfio.h | 11 +
> > > > include/uapi/linux/vhost.h | 10 +
> > > > include/uapi/linux/vhost_types.h | 5 +
> > > > 10 files changed, 573 insertions(+), 7 deletions(-)
> > > > create mode 100644 drivers/vhost/mdev.c
> > > >
^ permalink raw reply
* Re: [PATCH] bonding/802.3ad: fix slave initialization states race
From: Jay Vosburgh @ 2019-09-18 14:34 UTC (permalink / raw)
To: Aleksei Zakharov; +Cc: netdev
In-Reply-To: <20190918130545.GA11133@yandex.ru>
Aleksei Zakharov <zaharov@selectel.ru> wrote:
>Once a while, one of 802.3ad slaves fails to initialize and hangs in
>BOND_LINK_FAIL state. Commit 334031219a84 ("bonding/802.3ad: fix slave
>link initialization transition states") checks slave->last_link_up. But
>link can still hang in weird state.
>After physical link comes up it sends first two LACPDU messages and
>doesn't work properly after that. It doesn't send or receive LACPDU.
>Once it happens, the only message in dmesg is:
>bond1: link status up again after 0 ms for interface eth2
I believe this message indicates that the slave entered
BOND_LINK_FAIL state, but downdelay was not set. The _FAIL state is
really for managing the downdelay expiration, and a slave should not be
in that state (outside of a brief transition entirely within
bond_miimon_inspect) if downdelay is 0.
>This behavior can be reproduced (not every time):
>1. Set slave link down
>2. Wait for 1-3 seconds
>3. Set slave link up
>
>The fix is to check slave->link before setting it to BOND_LINK_FAIL or
>BOND_LINK_DOWN state. If got invalid Speed/Dupex values and link is in
>BOND_LINK_UP state, mark it as BOND_LINK_FAIL; otherwise mark it as
>BOND_LINK_DOWN.
>
>Fixes: 334031219a84 ("bonding/802.3ad: fix slave link initialization
>transition states")
>Signed-off-by: Aleksei Zakharov <zakharov.a.g@yandex.ru>
>---
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 931d9d935686..a28776d8f33f 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3135,7 +3135,7 @@ static int bond_slave_netdev_event(unsigned long event,
> */
> if (bond_update_speed_duplex(slave) &&
> BOND_MODE(bond) == BOND_MODE_8023AD) {
>- if (slave->last_link_up)
>+ if (slave->link == BOND_LINK_UP)
> slave->link = BOND_LINK_FAIL;
> else
> slave->link = BOND_LINK_DOWN;
Is the core problem here that slaves are reporting link up, but
returning invalid values for speed and/or duplex? If so, what network
device are you testing with that is exhibiting this behavior?
If I'm not mistaken, there have been several iterations of
hackery on this block of code to work around this same problem, and each
time there's some corner case that still doesn't work.
As Davem asked last time around, is the real problem that device
drivers report carrier up but supply invalid speed and duplex state?
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: dsa traffic priorization
From: Vladimir Oltean @ 2019-09-18 14:36 UTC (permalink / raw)
To: Sascha Hauer
Cc: netdev, Andrew Lunn, Vivien Didelot, Florian Fainelli, kernel
In-Reply-To: <20190918140225.imqchybuf3cnknob@pengutronix.de>
Hi Sascha,
On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> Hi All,
>
> We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
> regular network traffic on another port. The customer wants to configure two things
> on the switch: First Ethercat traffic shall be priorized over other network traffic
> (effectively prioritizing traffic based on port). Second the ethernet controller
> in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
> port shall be rate limited.
>
You probably already know this, but egress shaping will not drop
frames, just let them accumulate in the egress queue until something
else happens (e.g. queue occupancy threshold triggers pause frames, or
tail dropping is enabled, etc). Is this what you want? It sounds a bit
strange to me to configure egress shaping on the CPU port of a DSA
switch. That literally means you are buffering frames inside the
system. What about ingress policing?
> For reference the patch below configures the switch to their needs. Now the question
> is how this can be implemented in a way suitable for mainline. It looks like the per
> port priority mapping for VLAN tagged packets could be done via ip link add link ...
> ingress-qos-map QOS-MAP. How the default priority would be set is unclear to me.
>
Technically, configuring a match-all rxnfc rule with ethtool would
count as 'default priority' - I have proposed that before. Now I'm not
entirely sure how intuitive it is, but I'm also interested in being
able to configure this.
> The other part of the problem seems to be that the CPU port has no network device
> representation in Linux, so there's no interface to configure the egress limits via tc.
> This has been discussed before, but it seems there hasn't been any consensous regarding how
> we want to proceed?
>
> Sascha
>
> -----------------------------8<-----------------------------------
>
> drivers/net/dsa/mv88e6xxx/chip.c | 54 +++++++++++++++++++-
> drivers/net/dsa/mv88e6xxx/port.c | 87 ++++++++++++++++++++++++++++++++
> drivers/net/dsa/mv88e6xxx/port.h | 19 +++++++
> 3 files changed, 159 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index d0a97eb73a37..2a15cf259d04 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2090,7 +2090,9 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
> {
> struct dsa_switch *ds = chip->ds;
> int err;
> + u16 addr;
> u16 reg;
> + u16 val;
>
> chip->ports[port].chip = chip;
> chip->ports[port].port = port;
> @@ -2246,7 +2248,57 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
> /* Default VLAN ID and priority: don't set a default VLAN
> * ID, and set the default packet priority to zero.
> */
> - return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_DEFAULT_VLAN, 0);
> + err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_DEFAULT_VLAN, 0);
> + if (err)
> + return err;
> +
> +#define SWITCH_CPU_PORT 5
> +#define SWITCH_ETHERCAT_PORT 3
> +
> + /* set the egress rate */
> + switch (port) {
> + case SWITCH_CPU_PORT:
> + err = mv88e6xxx_port_set_egress_rate(chip, port,
> + MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME, 30000);
> + break;
> + default:
> + err = mv88e6xxx_port_set_egress_rate(chip, port,
> + MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME, 0);
> + break;
> + }
> +
> + if (err)
> + return err;
> +
> + /* set the output queue usage */
> + switch (port) {
> + case SWITCH_CPU_PORT:
> + err = mv88e6xxx_port_set_output_queue_schedule(chip, port,
> + MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_Q3_STRICT);
> + break;
> + default:
> + err = mv88e6xxx_port_set_output_queue_schedule(chip, port,
> + MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_NONE_STRICT);
> + break;
> + }
> +
> + if (err)
> + return err;
> +
> + /* set the default QPri */
> + switch (port) {
> + case SWITCH_ETHERCAT_PORT:
> + err = mv88e6xxx_port_set_default_qpri(chip, port, 3);
> + break;
> + default:
> + err = mv88e6xxx_port_set_default_qpri(chip, port, 2);
> + break;
> + }
> +
> + if (err)
> + return err;
> +
> + return 0;
> }
>
> static int mv88e6xxx_port_enable(struct dsa_switch *ds, int port,
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index 04309ef0a1cc..e03f24308f15 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -1147,6 +1147,22 @@ int mv88e6165_port_set_jumbo_size(struct mv88e6xxx_chip *chip, int port,
> return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL2, reg);
> }
>
> +int mv88e6xxx_port_set_default_qpri(struct mv88e6xxx_chip *chip, int port, int qpri)
> +{
> + u16 reg;
> + int err;
> +
> + err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL2, ®);
> + if (err)
> + return err;
> +
> + reg &= ~MV88E6XXX_PORT_CTL2_DEF_QPRI_MASK;
> + reg |= (qpri << 1) & MV88E6XXX_PORT_CTL2_DEF_QPRI_MASK;
> + reg |= MV88E6XXX_PORT_CTL2_USE_DEF_QPRI;
> +
> + return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL2, reg);
> +}
> +
> /* Offset 0x09: Port Rate Control */
>
> int mv88e6095_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port)
> @@ -1161,6 +1177,77 @@ int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port)
> 0x0001);
> }
>
> +int mv88e6xxx_port_set_output_queue_schedule(struct mv88e6xxx_chip *chip, int port,
> + u16 schedule)
> +{
> + u16 reg;
> + int err;
> +
> + err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, ®);
> + if (err)
> + return err;
> +
> + reg &= ~MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_MASK;
> + reg |= schedule;
> +
> + return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, reg);
> +}
> +
> +static int _mv88e6xxx_egress_rate_calc_frames(u32 rate, u16 *egress_rate_val)
> +{
> + const volatile u32 scale_factor = (1000 * 1000 * 1000);
> + volatile u32 u;
> +
> + if (rate > 1488000)
> + return EINVAL;
> +
> + if (rate < 7600)
> + return EINVAL;
> +
> + u = 32 * rate;
> + u = scale_factor / u; /* scale_factor used to convert 32s into 32ns */
> +
> + *egress_rate_val = (u16)u;
> +
> + return 0;
> +}
> +
> +int mv88e6xxx_port_set_egress_rate(struct mv88e6xxx_chip *chip, int port, u16 type,
> + u32 rate)
> +{
> + u16 reg;
> + int err;
> + u16 egress_rate_val;
> +
> + err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, ®);
> + if (err)
> + return err;
> +
> + reg &= ~MV88E6XXX_PORT_EGRESS_RATE_CTL2_RATE_MASK;
> +
> + if (rate) {
> + reg &= ~MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_MASK;
> + reg |= type;
> +
> + switch (type) {
> + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME:
> + err = _mv88e6xxx_egress_rate_calc_frames(rate, &egress_rate_val);
> + if (err)
> + return err;
> + reg |= egress_rate_val & 0x0FFF;
> + break;
> + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L1:
> + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L2:
> + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L3:
> + return EINVAL; /* ToDo */
> + default:
> + return EINVAL;
> + }
> + }
> +
> + return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, reg);
> +}
> +
> /* Offset 0x0C: Port ATU Control */
>
> int mv88e6xxx_port_disable_learn_limit(struct mv88e6xxx_chip *chip, int port)
> diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
> index 8d5a6cd6fb19..cdd057c52ab8 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.h
> +++ b/drivers/net/dsa/mv88e6xxx/port.h
> @@ -197,6 +197,8 @@
> #define MV88E6XXX_PORT_CTL2_DEFAULT_FORWARD 0x0040
> #define MV88E6XXX_PORT_CTL2_EGRESS_MONITOR 0x0020
> #define MV88E6XXX_PORT_CTL2_INGRESS_MONITOR 0x0010
> +#define MV88E6XXX_PORT_CTL2_USE_DEF_QPRI 0x0008
> +#define MV88E6XXX_PORT_CTL2_DEF_QPRI_MASK 0x0006
> #define MV88E6095_PORT_CTL2_CPU_PORT_MASK 0x000f
>
> /* Offset 0x09: Egress Rate Control */
> @@ -204,6 +206,17 @@
>
> /* Offset 0x0A: Egress Rate Control 2 */
> #define MV88E6XXX_PORT_EGRESS_RATE_CTL2 0x0a
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_MASK 0xC000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME 0x0000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L1 0x4000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L2 0x8000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L3 0xC000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_MASK 0x3000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_NONE_STRICT 0x0000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_Q3_STRICT 0x1000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_Q3_Q2_STRICT 0x2000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_ALL_STRICT 0x3000
> +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_RATE_MASK 0x0FFF
>
> /* Offset 0x0B: Port Association Vector */
> #define MV88E6XXX_PORT_ASSOC_VECTOR 0x0b
> @@ -326,8 +339,14 @@ int mv88e6xxx_port_set_message_port(struct mv88e6xxx_chip *chip, int port,
> bool message_port);
> int mv88e6165_port_set_jumbo_size(struct mv88e6xxx_chip *chip, int port,
> size_t size);
> +int mv88e6xxx_port_set_default_qpri(struct mv88e6xxx_chip *chip, int port,
> + int qpri);
> int mv88e6095_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
> int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
> +int mv88e6xxx_port_set_output_queue_schedule(struct mv88e6xxx_chip *chip, int port,
> + u16 schedule);
> +int mv88e6xxx_port_set_egress_rate(struct mv88e6xxx_chip *chip, int port,
> + u16 type, u32 rate);
> int mv88e6097_port_pause_limit(struct mv88e6xxx_chip *chip, int port, u8 in,
> u8 out);
> int mv88e6390_port_pause_limit(struct mv88e6xxx_chip *chip, int port, u8 in,
> --
> 2.23.0
>
> --
> 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 |
Regards,
-Vladimir
^ permalink raw reply
* [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable function
From: Peter Mamonov @ 2019-09-18 14:48 UTC (permalink / raw)
To: andrew; +Cc: Peter Mamonov, Florian Fainelli, netdev, linux-kernel
In-Reply-To: <20190918141931.GK9591@lunn.ch>
According to the DP83865 datasheet "The 10 Mbps HDX loopback can be
disabled in the expanded memory register 0x1C0.1." The driver erroneously
used bit 0 instead of bit 1.
Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
---
drivers/net/phy/national.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
index 2addf1d3f619..3aa910b3dc89 100644
--- a/drivers/net/phy/national.c
+++ b/drivers/net/phy/national.c
@@ -110,14 +110,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable)
{
+ u16 lb_dis = BIT(1);
+
if (disable)
- ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1);
+ ns_exp_write(phydev, 0x1c0,
+ ns_exp_read(phydev, 0x1c0) | lb_dis);
else
ns_exp_write(phydev, 0x1c0,
- ns_exp_read(phydev, 0x1c0) & 0xfffe);
+ ns_exp_read(phydev, 0x1c0) & ~lb_dis);
pr_debug("10BASE-T HDX loopback %s\n",
- (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on");
+ (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on");
}
static int ns_config_init(struct phy_device *phydev)
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] net: sysctl: cleanup net_sysctl_init error exit paths
From: George G. Davis @ 2019-09-18 15:00 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: David Miller, netdev, linux-kernel
In-Reply-To: <14606764-a026-c171-ba71-bf242a930e7e@6wind.com>
Hello Nicolas,
On Wed, Sep 18, 2019 at 11:44:55AM +0200, Nicolas Dichtel wrote:
> Le 17/09/2019 à 17:53, George G. Davis a écrit :
> [snip]
> > Ping, "Linux 5.3" kernel has been released [1] and it appears that the
> > 5.4 merge window is open. The patch [2] remains unchanged since my initial
> > post. Please consider applying it.
>
> net-next is closed:
> http://vger.kernel.org/~davem/net-next.html
>
> You will have to resend the patch when net-next opens.
Thanks! I'll watch for the "net-next is OPEN" announcment and resubmit then.
--
Regards,
George
^ permalink raw reply
* Re: dsa traffic priorization
From: Dave Taht @ 2019-09-18 15:03 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Sascha Hauer, netdev, Andrew Lunn, Vivien Didelot,
Florian Fainelli, kernel
In-Reply-To: <CA+h21hpG52R6ScGpGX86Q7MuRHCgGNY-TxzaQGu2wZR8EtPtbA@mail.gmail.com>
On Wed, Sep 18, 2019 at 7:37 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Sascha,
>
> On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > Hi All,
> >
> > We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
> > regular network traffic on another port. The customer wants to configure two things
> > on the switch: First Ethercat traffic shall be priorized over other network traffic
> > (effectively prioritizing traffic based on port). Second the ethernet controller
> > in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
> > port shall be rate limited.
> >
>
> You probably already know this, but egress shaping will not drop
> frames, just let them accumulate in the egress queue until something
> else happens (e.g. queue occupancy threshold triggers pause frames, or
> tail dropping is enabled, etc). Is this what you want? It sounds a bit
Dropping in general is a basic attribute of the fq_codel algorithm which is
enabled by default on many boxes. It's latency sensitive, so it responds well
to pause frame (over) use.
Usually the cpu to switch port is exposed via vlan (e.g eth0:2), and
while you can inbound and
outbound shape on that - using htb/hfsc + fq_codel, or cake
But, also, most usually what happens when the cpu cannot keep up with
the switch is we drop packets on the rx ring for receive, and in
fq-codel on send.
> strange to me to configure egress shaping on the CPU port of a DSA
> switch. That literally means you are buffering frames inside the
> system. What about ingress policing?
>
> > For reference the patch below configures the switch to their needs. Now the question
> > is how this can be implemented in a way suitable for mainline. It looks like the per
> > port priority mapping for VLAN tagged packets could be done via ip link add link ...
> > ingress-qos-map QOS-MAP. How the default priority would be set is unclear to me.
> >
>
> Technically, configuring a match-all rxnfc rule with ethtool would
> count as 'default priority' - I have proposed that before. Now I'm not
> entirely sure how intuitive it is, but I'm also interested in being
> able to configure this.
>
> > The other part of the problem seems to be that the CPU port has no network device
> > representation in Linux, so there's no interface to configure the egress limits via tc.
> > This has been discussed before, but it seems there hasn't been any consensous regarding how
> > we want to proceed?
> >
> > Sascha
> >
> > -----------------------------8<-----------------------------------
> >
> > drivers/net/dsa/mv88e6xxx/chip.c | 54 +++++++++++++++++++-
> > drivers/net/dsa/mv88e6xxx/port.c | 87 ++++++++++++++++++++++++++++++++
> > drivers/net/dsa/mv88e6xxx/port.h | 19 +++++++
> > 3 files changed, 159 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> > index d0a97eb73a37..2a15cf259d04 100644
> > --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -2090,7 +2090,9 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
> > {
> > struct dsa_switch *ds = chip->ds;
> > int err;
> > + u16 addr;
> > u16 reg;
> > + u16 val;
> >
> > chip->ports[port].chip = chip;
> > chip->ports[port].port = port;
> > @@ -2246,7 +2248,57 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
> > /* Default VLAN ID and priority: don't set a default VLAN
> > * ID, and set the default packet priority to zero.
> > */
> > - return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_DEFAULT_VLAN, 0);
> > + err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_DEFAULT_VLAN, 0);
> > + if (err)
> > + return err;
> > +
> > +#define SWITCH_CPU_PORT 5
> > +#define SWITCH_ETHERCAT_PORT 3
> > +
> > + /* set the egress rate */
> > + switch (port) {
> > + case SWITCH_CPU_PORT:
> > + err = mv88e6xxx_port_set_egress_rate(chip, port,
> > + MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME, 30000);
> > + break;
> > + default:
> > + err = mv88e6xxx_port_set_egress_rate(chip, port,
> > + MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME, 0);
> > + break;
> > + }
> > +
> > + if (err)
> > + return err;
> > +
> > + /* set the output queue usage */
> > + switch (port) {
> > + case SWITCH_CPU_PORT:
> > + err = mv88e6xxx_port_set_output_queue_schedule(chip, port,
> > + MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_Q3_STRICT);
> > + break;
> > + default:
> > + err = mv88e6xxx_port_set_output_queue_schedule(chip, port,
> > + MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_NONE_STRICT);
> > + break;
> > + }
> > +
> > + if (err)
> > + return err;
> > +
> > + /* set the default QPri */
> > + switch (port) {
> > + case SWITCH_ETHERCAT_PORT:
> > + err = mv88e6xxx_port_set_default_qpri(chip, port, 3);
> > + break;
> > + default:
> > + err = mv88e6xxx_port_set_default_qpri(chip, port, 2);
> > + break;
> > + }
> > +
> > + if (err)
> > + return err;
> > +
> > + return 0;
> > }
> >
> > static int mv88e6xxx_port_enable(struct dsa_switch *ds, int port,
> > diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> > index 04309ef0a1cc..e03f24308f15 100644
> > --- a/drivers/net/dsa/mv88e6xxx/port.c
> > +++ b/drivers/net/dsa/mv88e6xxx/port.c
> > @@ -1147,6 +1147,22 @@ int mv88e6165_port_set_jumbo_size(struct mv88e6xxx_chip *chip, int port,
> > return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL2, reg);
> > }
> >
> > +int mv88e6xxx_port_set_default_qpri(struct mv88e6xxx_chip *chip, int port, int qpri)
> > +{
> > + u16 reg;
> > + int err;
> > +
> > + err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL2, ®);
> > + if (err)
> > + return err;
> > +
> > + reg &= ~MV88E6XXX_PORT_CTL2_DEF_QPRI_MASK;
> > + reg |= (qpri << 1) & MV88E6XXX_PORT_CTL2_DEF_QPRI_MASK;
> > + reg |= MV88E6XXX_PORT_CTL2_USE_DEF_QPRI;
> > +
> > + return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL2, reg);
> > +}
> > +
> > /* Offset 0x09: Port Rate Control */
> >
> > int mv88e6095_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port)
> > @@ -1161,6 +1177,77 @@ int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port)
> > 0x0001);
> > }
> >
> > +int mv88e6xxx_port_set_output_queue_schedule(struct mv88e6xxx_chip *chip, int port,
> > + u16 schedule)
> > +{
> > + u16 reg;
> > + int err;
> > +
> > + err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, ®);
> > + if (err)
> > + return err;
> > +
> > + reg &= ~MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_MASK;
> > + reg |= schedule;
> > +
> > + return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, reg);
> > +}
> > +
> > +static int _mv88e6xxx_egress_rate_calc_frames(u32 rate, u16 *egress_rate_val)
> > +{
> > + const volatile u32 scale_factor = (1000 * 1000 * 1000);
> > + volatile u32 u;
> > +
> > + if (rate > 1488000)
> > + return EINVAL;
> > +
> > + if (rate < 7600)
> > + return EINVAL;
> > +
> > + u = 32 * rate;
> > + u = scale_factor / u; /* scale_factor used to convert 32s into 32ns */
> > +
> > + *egress_rate_val = (u16)u;
> > +
> > + return 0;
> > +}
> > +
> > +int mv88e6xxx_port_set_egress_rate(struct mv88e6xxx_chip *chip, int port, u16 type,
> > + u32 rate)
> > +{
> > + u16 reg;
> > + int err;
> > + u16 egress_rate_val;
> > +
> > + err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, ®);
> > + if (err)
> > + return err;
> > +
> > + reg &= ~MV88E6XXX_PORT_EGRESS_RATE_CTL2_RATE_MASK;
> > +
> > + if (rate) {
> > + reg &= ~MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_MASK;
> > + reg |= type;
> > +
> > + switch (type) {
> > + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME:
> > + err = _mv88e6xxx_egress_rate_calc_frames(rate, &egress_rate_val);
> > + if (err)
> > + return err;
> > + reg |= egress_rate_val & 0x0FFF;
> > + break;
> > + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L1:
> > + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L2:
> > + case MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L3:
> > + return EINVAL; /* ToDo */
> > + default:
> > + return EINVAL;
> > + }
> > + }
> > +
> > + return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2, reg);
> > +}
> > +
> > /* Offset 0x0C: Port ATU Control */
> >
> > int mv88e6xxx_port_disable_learn_limit(struct mv88e6xxx_chip *chip, int port)
> > diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
> > index 8d5a6cd6fb19..cdd057c52ab8 100644
> > --- a/drivers/net/dsa/mv88e6xxx/port.h
> > +++ b/drivers/net/dsa/mv88e6xxx/port.h
> > @@ -197,6 +197,8 @@
> > #define MV88E6XXX_PORT_CTL2_DEFAULT_FORWARD 0x0040
> > #define MV88E6XXX_PORT_CTL2_EGRESS_MONITOR 0x0020
> > #define MV88E6XXX_PORT_CTL2_INGRESS_MONITOR 0x0010
> > +#define MV88E6XXX_PORT_CTL2_USE_DEF_QPRI 0x0008
> > +#define MV88E6XXX_PORT_CTL2_DEF_QPRI_MASK 0x0006
> > #define MV88E6095_PORT_CTL2_CPU_PORT_MASK 0x000f
> >
> > /* Offset 0x09: Egress Rate Control */
> > @@ -204,6 +206,17 @@
> >
> > /* Offset 0x0A: Egress Rate Control 2 */
> > #define MV88E6XXX_PORT_EGRESS_RATE_CTL2 0x0a
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_MASK 0xC000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_FRAME 0x0000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L1 0x4000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L2 0x8000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_COUNT_MODE_L3 0xC000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_MASK 0x3000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_NONE_STRICT 0x0000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_Q3_STRICT 0x1000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_Q3_Q2_STRICT 0x2000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_ALL_STRICT 0x3000
> > +#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_RATE_MASK 0x0FFF
> >
> > /* Offset 0x0B: Port Association Vector */
> > #define MV88E6XXX_PORT_ASSOC_VECTOR 0x0b
> > @@ -326,8 +339,14 @@ int mv88e6xxx_port_set_message_port(struct mv88e6xxx_chip *chip, int port,
> > bool message_port);
> > int mv88e6165_port_set_jumbo_size(struct mv88e6xxx_chip *chip, int port,
> > size_t size);
> > +int mv88e6xxx_port_set_default_qpri(struct mv88e6xxx_chip *chip, int port,
> > + int qpri);
> > int mv88e6095_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
> > int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
> > +int mv88e6xxx_port_set_output_queue_schedule(struct mv88e6xxx_chip *chip, int port,
> > + u16 schedule);
> > +int mv88e6xxx_port_set_egress_rate(struct mv88e6xxx_chip *chip, int port,
> > + u16 type, u32 rate);
> > int mv88e6097_port_pause_limit(struct mv88e6xxx_chip *chip, int port, u8 in,
> > u8 out);
> > int mv88e6390_port_pause_limit(struct mv88e6xxx_chip *chip, int port, u8 in,
> > --
> > 2.23.0
> >
> > --
> > 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 |
>
> Regards,
> -Vladimir
--
Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740
^ permalink raw reply
* [PATCH net] sch_netem: fix a divide by zero in tabledist()
From: Eric Dumazet @ 2019-09-18 15:05 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, syzbot
syzbot managed to crash the kernel in tabledist() loading
an empty distribution table.
t = dist->table[rnd % dist->size];
Simply return an error when such load is attempted.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
net/sched/sch_netem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index b17f2ed970e296adc57bed458ec3cced4fc6705b..f5cb35e550f8df557f2e444cc2fd142cab97789b 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -777,7 +777,7 @@ static int get_dist_table(struct Qdisc *sch, struct disttable **tbl,
struct disttable *d;
int i;
- if (n > NETEM_DIST_MAX)
+ if (!n || n > NETEM_DIST_MAX)
return -EINVAL;
d = kvmalloc(sizeof(struct disttable) + n * sizeof(s16), GFP_KERNEL);
--
2.23.0.237.gc6a4ce50a0-goog
^ permalink raw reply related
* Re: [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable function
From: Andrew Lunn @ 2019-09-18 15:26 UTC (permalink / raw)
To: Peter Mamonov; +Cc: Florian Fainelli, netdev, linux-kernel
In-Reply-To: <20190918144825.23285-1-pmamonov@gmail.com>
On Wed, Sep 18, 2019 at 05:48:25PM +0300, Peter Mamonov wrote:
> According to the DP83865 datasheet "The 10 Mbps HDX loopback can be
> disabled in the expanded memory register 0x1C0.1." The driver erroneously
> used bit 0 instead of bit 1.
Hi Peter
This is version 2, not 1. Or if you want to start counting from 0, it
would be good to put v0 in your first patch :-)
It is also normal to put in the commit message what changed from the
previous version.
This is a fix. So please add a Fixes: tag, with the hash of the commit
which introduced the problem.
And since this is a fix, it should be against DaveM net tree, and you
indicate this in the subject line with [PATCH net v3].
Thanks
Andrew
>
> Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
> ---
> drivers/net/phy/national.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
> index 2addf1d3f619..3aa910b3dc89 100644
> --- a/drivers/net/phy/national.c
> +++ b/drivers/net/phy/national.c
> @@ -110,14 +110,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
>
> static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable)
> {
> + u16 lb_dis = BIT(1);
> +
> if (disable)
> - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1);
> + ns_exp_write(phydev, 0x1c0,
> + ns_exp_read(phydev, 0x1c0) | lb_dis);
> else
> ns_exp_write(phydev, 0x1c0,
> - ns_exp_read(phydev, 0x1c0) & 0xfffe);
> + ns_exp_read(phydev, 0x1c0) & ~lb_dis);
>
> pr_debug("10BASE-T HDX loopback %s\n",
> - (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on");
> + (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on");
> }
>
> static int ns_config_init(struct phy_device *phydev)
> --
> 2.23.0
>
^ permalink raw reply
* Re: udp sendmsg ENOBUFS clarification
From: Willem de Bruijn @ 2019-09-18 15:35 UTC (permalink / raw)
To: Josh Hunt; +Cc: netdev, Eric Dumazet, David Miller
In-Reply-To: <ce01f024-268d-a44e-8093-91be97f1e8b0@akamai.com>
On Tue, Sep 17, 2019 at 4:20 PM Josh Hunt <johunt@akamai.com> wrote:
>
> I was running some tests recently with the udpgso_bench_tx benchmark in
> selftests and noticed that in some configurations it reported sending
> more than line rate! Looking into it more I found that I was overflowing
> the qdisc queue and so it was sending back NET_XMIT_DROP however this
> error did not propagate back up to the application and so it assumed
> whatever it sent was done successfully. That's when I learned about
> IP_RECVERR and saw that the benchmark isn't using that socket option.
>
> That's all fairly straightforward, but what I was hoping to get
> clarification on is where is the line drawn on when or when not to send
> ENOBUFS back to the application if IP_RECVERR is *not* set? My guess
> based on going through the code is that as long as the packet leaves the
> stack (in this case sent to the qdisc) that's where we stop reporting
> ENOBUFS back to the application, but can someone confirm?
Once a packet is queued the system call may return, so any subsequent
drops after dequeue are not propagated back. The relevant rc is set in
__dev_xmit_skb on q->enqueue. On setups with multiple devices, such as
a tunnel or bonding path, enqueue on the lower device is similar not
propagated.
> For example, we sanitize the error in udp_send_skb():
> send:
> err = ip_send_skb(sock_net(sk), skb);
> if (err) {
> if (err == -ENOBUFS && !inet->recverr) {
> UDP_INC_STATS(sock_net(sk),
> UDP_MIB_SNDBUFERRORS, is_udplite);
> err = 0;
> }
> } else
>
>
> but in udp_sendmsg() we don't:
>
> if (err == -ENOBUFS || test_bit(SOCK_NOSPACE,
> &sk->sk_socket->flags)) {
> UDP_INC_STATS(sock_net(sk),
> UDP_MIB_SNDBUFERRORS, is_udplite);
> }
> return err;
That's interesting. My --incorrect-- understanding until now had been
that IP_RECVERR does nothing but enable optional extra detailed error
reporting on top of system call error codes.
But indeed it enables backpressure being reported as a system call
error that is suppressed otherwise. I don't know why. The behavior
precedes git history.
> In the case above it looks like we may only get ENOBUFS for allocation
> failures inside of the stack in udp_sendmsg() and so that's why we
> propagate the error back up to the application?
Both the udp lockless fast path and the slow corked path go through
udp_send_skb, so the backpressure is suppressed consistently across
both cases.
Indeed the error handling in udp_sendmsg then is not related to
backpressure, but to other causes of ENOBUF, i.e., allocation failure.
^ permalink raw reply
* Re: [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable function
From: Peter Mamonov @ 2019-09-18 15:40 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, netdev, linux-kernel
In-Reply-To: <20190918152646.GL9591@lunn.ch>
Hi Andrew,
On Wed, Sep 18, 2019 at 05:26:46PM +0200, Andrew Lunn wrote:
> On Wed, Sep 18, 2019 at 05:48:25PM +0300, Peter Mamonov wrote:
> > According to the DP83865 datasheet "The 10 Mbps HDX loopback can be
> > disabled in the expanded memory register 0x1C0.1." The driver erroneously
> > used bit 0 instead of bit 1.
>
> Hi Peter
>
> This is version 2, not 1. Or if you want to start counting from 0, it
> would be good to put v0 in your first patch :-)
>
> It is also normal to put in the commit message what changed from the
> previous version.
>
> This is a fix. So please add a Fixes: tag, with the hash of the commit
> which introduced the problem.
>
> And since this is a fix, it should be against DaveM net tree, and you
> indicate this in the subject line with [PATCH net v3].
Thanks for the tips! Will submit new version soon.
Regards,
Peter
>
> Thanks
> Andrew
>
> >
> > Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
> > ---
> > drivers/net/phy/national.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
> > index 2addf1d3f619..3aa910b3dc89 100644
> > --- a/drivers/net/phy/national.c
> > +++ b/drivers/net/phy/national.c
> > @@ -110,14 +110,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
> >
> > static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable)
> > {
> > + u16 lb_dis = BIT(1);
> > +
> > if (disable)
> > - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1);
> > + ns_exp_write(phydev, 0x1c0,
> > + ns_exp_read(phydev, 0x1c0) | lb_dis);
> > else
> > ns_exp_write(phydev, 0x1c0,
> > - ns_exp_read(phydev, 0x1c0) & 0xfffe);
> > + ns_exp_read(phydev, 0x1c0) & ~lb_dis);
> >
> > pr_debug("10BASE-T HDX loopback %s\n",
> > - (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on");
> > + (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on");
> > }
> >
> > static int ns_config_init(struct phy_device *phydev)
> > --
> > 2.23.0
> >
^ permalink raw reply
* Re: [PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature
From: Willem de Bruijn @ 2019-09-18 16:10 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Network Development, Paolo Abeni
In-Reply-To: <20190918072517.16037-3-steffen.klassert@secunet.com>
On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> This adds a new NETIF_F_GRO_LIST feature flag. I will be used
> to configure listfyed GRO what will be implemented with some
> followup paches.
This should probably simultaneously introduce SKB_GSO_FRAGLIST as well
as a BUILD_BUG_ON in net_gso_ok.
Please also in the commit describe the constraints of skbs that have
this type. If I'm not mistaken, an skb with either gso_size linear
data or one gso_sized frag, followed by a frag_list of the same. With
the exception of the last frag_list member, whose mss may be less than
gso_size. This will help when reasoning about all the types of skbs we
may see at segmentation, as we recently had to do [1]
Minor nit: I think it's listified, not listifyed.
[1] https://lore.kernel.org/netdev/CA+FuTSfVsgNDi7c=GUU8nMg2hWxF2SjCNLXetHeVPdnxAW5K-w@mail.gmail.com/
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> include/linux/netdev_features.h | 2 ++
> net/core/ethtool.c | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 4b19c544c59a..1b6baa1b6fe9 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -80,6 +80,7 @@ enum {
>
> NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
> + NETIF_F_GRO_LIST_BIT, /* Listifyed GRO */
>
> /*
> * Add your fresh new feature above and remember to update
> @@ -150,6 +151,7 @@ enum {
> #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
> #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
> #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
> +#define NETIF_F_GRO_LIST __NETIF_F(GRO_LIST)
>
> /* Finds the next feature with the highest number of the range of start till 0.
> */
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 6288e69e94fc..ee8d2b58c2d7 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
> [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
> + [NETIF_F_GRO_LIST_BIT] = "rx-gro-list",
> };
>
> static const char
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH RFC v3 5/5] udp: Support UDP fraglist GRO/GSO.
From: Willem de Bruijn @ 2019-09-18 16:13 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Network Development, Paolo Abeni
In-Reply-To: <20190918072517.16037-6-steffen.klassert@secunet.com>
On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> This patch extends UDP GRO to support fraglist GRO/GSO
> by using the previously introduced infrastructure.
> All UDP packets that are not targeted to a GRO capable
> UDP sockets are going to fraglist GRO now (local input
> and forward).
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> @@ -419,7 +460,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> (skb->ip_summed != CHECKSUM_PARTIAL &&
> NAPI_GRO_CB(skb)->csum_cnt == 0 &&
> !NAPI_GRO_CB(skb)->csum_valid))
> - goto out_unlock;
> + goto out;
>
> /* mark that this skb passed once through the tunnel gro layer */
> NAPI_GRO_CB(skb)->encap_mark = 1;
> @@ -446,8 +487,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
>
> -out_unlock:
> - rcu_read_unlock();
> +out:
> skb_gro_flush_final(skb, pp, flush);
> return pp;
> }
This probably belongs in patch 1?
^ permalink raw reply
* Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO
From: Willem de Bruijn @ 2019-09-18 16:17 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Network Development, Paolo Abeni, marcelo.leitner
In-Reply-To: <20190918072517.16037-1-steffen.klassert@secunet.com>
On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> This patchset adds support to do GRO/GSO by chaining packets
> of the same flow at the SKB frag_list pointer. This avoids
> the overhead to merge payloads into one big packet, and
> on the other end, if GSO is needed it avoids the overhead
> of splitting the big packet back to the native form.
>
> Patch 1 Enables UDP GRO by default.
>
> Patch 2 adds a netdev feature flag to enable listifyed GRO,
> this implements one of the configuration options discussed
> at netconf 2019.
>
> Patch 3 adds a netdev software feature set that defaults to off
> and assigns the new listifyed GRO feature flag to it.
>
> Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
>
> Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> GRO supported socket is found.
Very nice feature, Steffen. Aside from questions around performance,
my only question is really how this relates to GSO_BY_FRAGS.
More specifically, whether we can remove that in favor of using your
new skb_segment_list. That would actually be a big first step in
simplifying skb_segment back to something manageable.
^ permalink raw reply
* [PATCH net v3] net/phy: fix DP83865 10 Mbps HDX loopback disable function
From: Peter Mamonov @ 2019-09-18 16:27 UTC (permalink / raw)
To: andrew
Cc: Peter Mamonov, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev, linux-kernel
According to the DP83865 datasheet "the 10 Mbps HDX loopback can be
disabled in the expanded memory register 0x1C0.1". The driver erroneously
used bit 0 instead of bit 1.
Fixes: 4621bf129856 ("phy: Add file missed in previous commit.")
Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
---
Changes since v2:
- None
Changes since v1:
- use BIT() macro
- fix debug message as well
drivers/net/phy/national.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
index a221dd552c3c..a5bf0874c7d8 100644
--- a/drivers/net/phy/national.c
+++ b/drivers/net/phy/national.c
@@ -105,14 +105,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable)
{
+ u16 lb_dis = BIT(1);
+
if (disable)
- ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1);
+ ns_exp_write(phydev, 0x1c0,
+ ns_exp_read(phydev, 0x1c0) | lb_dis);
else
ns_exp_write(phydev, 0x1c0,
- ns_exp_read(phydev, 0x1c0) & 0xfffe);
+ ns_exp_read(phydev, 0x1c0) & ~lb_dis);
pr_debug("10BASE-T HDX loopback %s\n",
- (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on");
+ (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on");
}
static int ns_config_init(struct phy_device *phydev)
--
2.23.0
^ permalink raw reply related
* Re: [PATCH net v3] net/phy: fix DP83865 10 Mbps HDX loopback disable function
From: Andrew Lunn @ 2019-09-18 16:32 UTC (permalink / raw)
To: Peter Mamonov
Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20190918162755.24024-1-pmamonov@gmail.com>
On Wed, Sep 18, 2019 at 07:27:55PM +0300, Peter Mamonov wrote:
> According to the DP83865 datasheet "the 10 Mbps HDX loopback can be
> disabled in the expanded memory register 0x1C0.1". The driver erroneously
> used bit 0 instead of bit 1.
>
> Fixes: 4621bf129856 ("phy: Add file missed in previous commit.")
> Signed-off-by: Peter Mamonov <pmamonov@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH] drivers/net/fjes: fix a potential NULL pointer dereference
From: Allen Pais @ 2019-09-18 16:33 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-kernel
alloc_workqueue is not checked for errors and as a result,
a potential NULL dereference could occur.
Signed-off-by: Allen Pais <allen.pais@oracle.com>
---
drivers/net/fjes/fjes_main.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
index bbbc1dc..2d04104 100644
--- a/drivers/net/fjes/fjes_main.c
+++ b/drivers/net/fjes/fjes_main.c
@@ -1237,8 +1237,15 @@ static int fjes_probe(struct platform_device *plat_dev)
adapter->open_guard = false;
adapter->txrx_wq = alloc_workqueue(DRV_NAME "/txrx", WQ_MEM_RECLAIM, 0);
+ if (unlikely(!adapter->txrx_wq))
+ goto err_free_netdev;
+
adapter->control_wq = alloc_workqueue(DRV_NAME "/control",
WQ_MEM_RECLAIM, 0);
+ if (unlikely(!adapter->control_wq)) {
+ destroy_workqueue(adapter->txrx_wq);
+ goto err_free_netdev;
+ }
INIT_WORK(&adapter->tx_stall_task, fjes_tx_stall_task);
INIT_WORK(&adapter->raise_intr_rxdata_task,
--
1.9.1
^ permalink raw reply related
* [PATCH] libertas: fix a potential NULL pointer dereference
From: Allen Pais @ 2019-09-18 16:35 UTC (permalink / raw)
To: netdev; +Cc: davem, kvalo, libertas-dev, linux-wireless, linux-kernel
alloc_workqueue is not checked for errors and as a result,
a potential NULL dereference could occur.
Signed-off-by: Allen Pais <allen.pais@oracle.com>
---
drivers/net/wireless/marvell/libertas/if_sdio.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/marvell/libertas/if_sdio.c b/drivers/net/wireless/marvell/libertas/if_sdio.c
index 242d884..30f1025 100644
--- a/drivers/net/wireless/marvell/libertas/if_sdio.c
+++ b/drivers/net/wireless/marvell/libertas/if_sdio.c
@@ -1179,6 +1179,10 @@ static int if_sdio_probe(struct sdio_func *func,
spin_lock_init(&card->lock);
card->workqueue = alloc_workqueue("libertas_sdio", WQ_MEM_RECLAIM, 0);
+ if (unlikely(!card->workqueue)) {
+ ret = -ENOMEM;
+ goto err_queue;
+ }
INIT_WORK(&card->packet_worker, if_sdio_host_to_card_worker);
init_waitqueue_head(&card->pwron_waitq);
@@ -1230,6 +1234,7 @@ static int if_sdio_probe(struct sdio_func *func,
lbs_remove_card(priv);
free:
destroy_workqueue(card->workqueue);
+err_queue:
while (card->packets) {
packet = card->packets;
card->packets = card->packets->next;
--
1.9.1
^ permalink raw reply related
* Re: ip netns exec hides mount points from child processes
From: Naja Melan @ 2019-09-18 16:26 UTC (permalink / raw)
To: netdev; +Cc: Eric W. Biederman
In-Reply-To: <871s5qdgqj.fsf@xmission.com>
Thank you for looking into it. Sorry for the long delay, but my answers got blocked by xmission several times and I think I just gave up on containers at some point. This is still not solved though. To answer Eric's question:
> Why you don't see the new sysfs is something I need more information to understand.
> Since everything else is mounted on top of sysfs. The code probably needs an update to bind mount (cgroups, debugfs, configs, pstore, selinuxfs, and securitfs) from the old sysfs to the new sysfs. That everything now gets mount points on sysfs is new from the time the code was written and the code just needs an update for that.
There is a mount of type sysfs. It is now called after the network namespace rather than being called sysfs, that's why I missed it last time. It looks like:
testns on /sys type sysfs (rw,relatime)
So that's probably not a problem.
So it seems the code of `ip netns exec` still hasn't been updated not to lose cgroups and all other system mounts...
I just checked with 5.2.14-arch2-1-ARCH
Thanks in advance,
Naja Melan
^ permalink raw reply
* Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO
From: Marcelo Ricardo Leitner @ 2019-09-18 16:58 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Steffen Klassert, Network Development, Paolo Abeni
In-Reply-To: <CA+FuTSdVFguDHXYPJBRrLhzPWBaykd+7PRqEmGf_eOFC3iHpAg@mail.gmail.com>
On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patchset adds support to do GRO/GSO by chaining packets
> > of the same flow at the SKB frag_list pointer. This avoids
> > the overhead to merge payloads into one big packet, and
> > on the other end, if GSO is needed it avoids the overhead
> > of splitting the big packet back to the native form.
> >
> > Patch 1 Enables UDP GRO by default.
> >
> > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > this implements one of the configuration options discussed
> > at netconf 2019.
> >
> > Patch 3 adds a netdev software feature set that defaults to off
> > and assigns the new listifyed GRO feature flag to it.
> >
> > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> >
> > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > GRO supported socket is found.
>
> Very nice feature, Steffen. Aside from questions around performance,
> my only question is really how this relates to GSO_BY_FRAGS.
They do the exact same thing AFAICT: they GSO according to a
pre-formatted list of fragments/packets, and not to a specific size
(such as MSS).
>
> More specifically, whether we can remove that in favor of using your
> new skb_segment_list. That would actually be a big first step in
> simplifying skb_segment back to something manageable.
The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
dealing with frags instead of frag_list was considered easier to be
offloaded, if ever attempted. So this would be a step back on that
aspect. Other than this, it should be doable.
^ permalink raw reply
* [PATCH] net: dsa: sja1105: prevent leaking memory
From: Navid Emamdoost @ 2019-09-18 17:10 UTC (permalink / raw)
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Vladimir Oltean,
Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
linux-kernel, netdev
In sja1105_static_config_upload, in two cases memory is leaked: when
static_config_buf_prepare_for_upload fails and when sja1105_inhibit_tx
fails. In both cases config_buf should be released.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_spi.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 84dc603138cf..80e86c714efb 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -408,8 +408,9 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
rc = static_config_buf_prepare_for_upload(priv, config_buf, buf_len);
if (rc < 0) {
- dev_err(dev, "Invalid config, cannot upload\n");
- return -EINVAL;
+ dev_err(dev, "Invalid config, cannot upload\n");
+ rc = -EINVAL;
+ goto out;
}
/* Prevent PHY jabbering during switch reset by inhibiting
* Tx on all ports and waiting for current packet to drain.
@@ -418,7 +419,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
rc = sja1105_inhibit_tx(priv, port_bitmap, true);
if (rc < 0) {
dev_err(dev, "Failed to inhibit Tx on ports\n");
- return -ENXIO;
+ rc = -ENXIO;
+ goto out;
}
/* Wait for an eventual egress packet to finish transmission
* (reach IFG). It is guaranteed that a second one will not
--
2.17.1
^ permalink raw reply related
* RE: [PATCH v3 0/5] Introduce variable length mdev alias
From: Parav Pandit @ 2019-09-18 17:15 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190917121357.02480c09.cohuck@redhat.com>
Hi Cornelia,
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, September 17, 2019 5:14 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v3 0/5] Introduce variable length mdev alias
>
> On Sun, 1 Sep 2019 23:24:31 -0500
> Parav Pandit <parav@mellanox.com> wrote:
>
> > To have consistent naming for the netdevice of a mdev and to have
> > consistent naming of the devlink port [1] of a mdev, which is formed
> > using phys_port_name of the devlink port, current UUID is not usable
> > because UUID is too long.
> >
> > UUID in string format is 36-characters long and in binary 128-bit.
> > Both formats are not able to fit within 15 characters limit of netdev
> > name.
> >
> > It is desired to have mdev device naming consistent using UUID.
> > So that widely used user space framework such as ovs [2] can make use
> > of mdev representor in similar way as PCIe SR-IOV VF and PF representors.
> >
> > Hence,
> > (a) mdev alias is created which is derived using sha1 from the mdev name.
> > (b) Vendor driver describes how long an alias should be for the child
> > mdev created for a given parent.
> > (c) Mdev aliases are unique at system level.
> > (d) alias is created optionally whenever parent requested.
> > This ensures that non networking mdev parents can function without
> > alias creation overhead.
> >
> > This design is discussed at [3].
> >
> > An example systemd/udev extension will have,
> >
> > 1. netdev name created using mdev alias available in sysfs.
> >
> > mdev UUID=83b8f4f2-509f-382f-3c1e-e6bfe0fa1001
> > mdev 12 character alias=cd5b146a80a5
> >
> > netdev name of this mdev = enmcd5b146a80a5 Here en = Ethernet link m =
> > mediated device
> >
> > 2. devlink port phys_port_name created using mdev alias.
> > devlink phys_port_name=pcd5b146a80a5
> >
> > This patchset enables mdev core to maintain unique alias for a mdev.
> >
> > Patch-1 Introduces mdev alias using sha1.
> > Patch-2 Ensures that mdev alias is unique in a system.
> > Patch-3 Exposes mdev alias in a sysfs hirerchy, update Documentation
> > Patch-4 Introduces mdev_alias() API.
> > Patch-5 Extends mtty driver to optionally provide alias generation.
> > This also enables to test UUID based sha1 collision and trigger error
> > handling for duplicate sha1 results.
> >
> > [1] http://man7.org/linux/man-pages/man8/devlink-port.8.html
> > [2] https://docs.openstack.org/os-vif/latest/user/plugins/ovs.html
> > [3] https://patchwork.kernel.org/cover/11084231/
> >
> > ---
> > Changelog:
> > v2->v3:
> > - Addressed comment from Yunsheng Lin
> > - Changed strcmp() ==0 to !strcmp()
> > - Addressed comment from Cornelia Hunk
> > - Merged sysfs Documentation patch with syfs patch
> > - Added more description for alias return value
> > v1->v2:
> > - Corrected a typo from 'and' to 'an'
> > - Addressed comments from Alex Williamson
> > - Kept mdev_device naturally aligned
> > - Added error checking for crypt_*() calls
> > - Moved alias NULL check at beginning
> > - Added mdev_alias() API
> > - Updated mtty driver to show example mdev_alias() usage
> > - Changed return type of generate_alias() from int to char*
> > v0->v1:
> > - Addressed comments from Alex Williamson, Cornelia Hunk and Mark
> > Bloch
> > - Moved alias length check outside of the parent lock
> > - Moved alias and digest allocation from kvzalloc to kzalloc
> > - &alias[0] changed to alias
> > - alias_length check is nested under get_alias_length callback check
> > - Changed comments to start with an empty line
> > - Added comment where alias memory ownership is handed over to mdev
> > device
> > - Fixed cleaunup of hash if mdev_bus_register() fails
> > - Updated documentation for new sysfs alias file
> > - Improved commit logs to make description more clear
> > - Fixed inclusiong of alias for NULL check
> > - Added ratelimited debug print for sha1 hash collision error
> >
> > Parav Pandit (5):
> > mdev: Introduce sha1 based mdev alias
> > mdev: Make mdev alias unique among all mdevs
> > mdev: Expose mdev alias in sysfs tree
> > mdev: Introduce an API mdev_alias
> > mtty: Optionally support mtty alias
> >
> > .../driver-api/vfio-mediated-device.rst | 9 ++
> > drivers/vfio/mdev/mdev_core.c | 142 +++++++++++++++++-
> > drivers/vfio/mdev/mdev_private.h | 5 +-
> > drivers/vfio/mdev/mdev_sysfs.c | 26 +++-
> > include/linux/mdev.h | 5 +
> > samples/vfio-mdev/mtty.c | 13 ++
> > 6 files changed, 190 insertions(+), 10 deletions(-)
> >
>
> The patches on their own look sane (and I gave my R-b), but the consumer of
> this new API should be ready before this is merged, as already discussed below.
Thanks for the review. I will send v4 here to address all comments and to add your R-b tag.
I am waiting for Saeed to post other prep series of mlx5_core to be merged before I post actual consumer series, as it depends on it.
I will also drop the mtty sample patch and change-log to avoid confusion with versions when I combine them with consumer series.
^ permalink raw reply
* Re: [PATCH] net: dsa: sja1105: prevent leaking memory
From: Andrew Lunn @ 2019-09-18 17:21 UTC (permalink / raw)
To: Navid Emamdoost
Cc: emamd001, smccaman, kjlu, Vladimir Oltean, Vivien Didelot,
Florian Fainelli, David S. Miller, linux-kernel, netdev
In-Reply-To: <20190918171020.5745-1-navid.emamdoost@gmail.com>
On Wed, Sep 18, 2019 at 12:10:19PM -0500, Navid Emamdoost wrote:
> In sja1105_static_config_upload, in two cases memory is leaked: when
> static_config_buf_prepare_for_upload fails and when sja1105_inhibit_tx
> fails. In both cases config_buf should be released.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Hi Navid
Please could you provide a Fixes: tag for where this memory leak was
introduced.
> ---
> drivers/net/dsa/sja1105/sja1105_spi.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
> index 84dc603138cf..80e86c714efb 100644
> --- a/drivers/net/dsa/sja1105/sja1105_spi.c
> +++ b/drivers/net/dsa/sja1105/sja1105_spi.c
> @@ -408,8 +408,9 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
>
> rc = static_config_buf_prepare_for_upload(priv, config_buf, buf_len);
> if (rc < 0) {
> - dev_err(dev, "Invalid config, cannot upload\n");
> - return -EINVAL;
> + dev_err(dev, "Invalid config, cannot upload\n");
What changed in this dev_err() call?
Thanks
Andrew
^ permalink raw reply
* [PATCH] net: qrtr: Stop rx_worker before freeing node
From: Bjorn Andersson @ 2019-09-18 17:21 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, linux-kernel, linux-arm-msm, Srinivas Kandagatla, stable
As the endpoint is unregistered there might still be work pending to
handle incoming messages, which will result in a use after free
scenario. The plan is to remove the rx_worker, but until then (and for
stable@) ensure that the work is stopped before the node is freed.
Fixes: bdabad3e363d ("net: Add Qualcomm IPC router")
Cc: stable@vger.kernel.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
net/qrtr/qrtr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 6c8b0f6d28f9..88f98f27ad88 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -150,6 +150,7 @@ static void __qrtr_node_release(struct kref *kref)
list_del(&node->item);
mutex_unlock(&qrtr_node_lock);
+ cancel_work_sync(&node->work);
skb_queue_purge(&node->rx_queue);
kfree(node);
}
--
2.18.0
^ permalink raw reply related
* Re: dsa traffic priorization
From: Florian Fainelli @ 2019-09-18 17:27 UTC (permalink / raw)
To: Dave Taht, Vladimir Oltean
Cc: Sascha Hauer, netdev, Andrew Lunn, Vivien Didelot, kernel
In-Reply-To: <CAA93jw6xB5uv48nB_rgtsky3mGthU2cjMMhuK_NFQeBxio4q5Q@mail.gmail.com>
On 9/18/19 8:03 AM, Dave Taht wrote:
> On Wed, Sep 18, 2019 at 7:37 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>>
>> Hi Sascha,
>>
>> On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>>>
>>> Hi All,
>>>
>>> We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
>>> regular network traffic on another port. The customer wants to configure two things
>>> on the switch: First Ethercat traffic shall be priorized over other network traffic
>>> (effectively prioritizing traffic based on port). Second the ethernet controller
>>> in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
>>> port shall be rate limited.
>>>
>>
>> You probably already know this, but egress shaping will not drop
>> frames, just let them accumulate in the egress queue until something
>> else happens (e.g. queue occupancy threshold triggers pause frames, or
>> tail dropping is enabled, etc). Is this what you want? It sounds a bit
>
> Dropping in general is a basic attribute of the fq_codel algorithm which is
> enabled by default on many boxes. It's latency sensitive, so it responds well
> to pause frame (over) use.
>
> Usually the cpu to switch port is exposed via vlan (e.g eth0:2), and
> while you can inbound and
> outbound shape on that - using htb/hfsc + fq_codel, or cake
That may be true with swconfig in OpenWrt, but this is not true with DSA
unless DSA_TAG_PROTO_8021Q is used which happens to be on just one
driver at the moment. With other switches that support a proprietary
switch tag format, there is not a particular VLAN or even a network
interface that describes the CPU port, other than the DSA master network
device which is the side facing the host system (not the switch itself).
>
> But, also, most usually what happens when the cpu cannot keep up with
> the switch is we drop packets on the rx ring for receive, and in
> fq-codel on send.
Dave, you seem to have a tendency to just pattern match on specific QoS-
related topics appearing on netdev and throwing the wonderful tool that
fq_codel without necessarily considering whether this is applicable or
not to the people raising the questions.
Since we are talking about hardware switches here and not simply
stations on a network (although the Ethernet MAC behind the CPU port
ends up being one), there is the possibility of using the HW to do
ingress and/or egress policing. The question raised by Sascha is how to
avoid statically configuring and instead using possibly existing tools
to achieve the same configuration, from user-space, that is, not encode
policy in the driver, but just the mechanism.
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox