* Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Pavel Machek @ 2017-09-08 8:56 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411219FF@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 970 bytes --]
On Thu 2017-09-07 21:08:58, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Break ksz_common.c into 2 files so that the common code can be used by other KSZ switch drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
> index ed335e2..0961c30 100644
> --- a/drivers/net/dsa/microchip/Makefile
> +++ b/drivers/net/dsa/microchip/Makefile
> @@ -1,2 +1,2 @@
> -obj-$(CONFIG_MICROCHIP_KSZ) += ksz_common.o
> +obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
I believe you should also rename option to CONFIG_MICROCHIP_KSZ_9477
here... and introduce appropriate Kconfig change.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Pavel Machek @ 2017-09-08 8:57 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121AFB@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]
> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > index ed335e2..0961c30 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,2 @@
> > > -obj-$(CONFIG_MICROCHIP_KSZ) += ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> >
> > Hi Tristram
> >
> > I would of thought this would break the build. You don't add ksz9477.c until the
> > next patch.
> >
> > Each patch needs to compile, otherwise you break git bisect.
> >
> > Andrew
>
> Eventually the file will need to be broken in two, so you would like
>to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1
> patch file?
Yes please.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added
From: Pavel Machek @ 2017-09-08 9:04 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A5B@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]
On Thu 2017-09-07 21:11:45, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add other KSZ switches support so that patch check does not complain.
Nice; unlike Andrew I believe that adding compatible strings is ok
before the support is added. Device tree description is
Linux-independend.
But... you need to Cc: device tree maintainers on this one. They
usually take some time to respond, so it is nice to do this early.
> index 0ab8b39..34af0e0 100644
> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
>
> Required properties:
>
> -- compatible: For external switch chips, compatible string must be exactly one
> - of: "microchip,ksz9477"
> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
> + "microchip,ksz8795" for KSZ8795 chip,
> + "microchip,ksz8794" for KSZ8794 chip,
> + "microchip,ksz8765" for KSZ8765 chip,
> + "microchip,ksz8895" for KSZ8895 chip,
> + "microchip,ksz8864" for KSZ8864 chip,
> + "microchip,ksz8873" for KSZ8873 chip,
> + "microchip,ksz8863" for KSZ8863 chip,
> + "microchip,ksz8463" for KSZ8463 chip
>
> See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties.
And you may want to wrap long line here, unless that's email client
playing another trick.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
From: Pavel Machek @ 2017-09-08 9:06 UTC (permalink / raw)
To: Andrew Lunn
Cc: Tristram.Ha, muvarov, nathan.leigh.conrad, vivien.didelot,
f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907224309.GZ11248@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1895 bytes --]
On Fri 2017-09-08 00:43:09, Andrew Lunn wrote:
> On Thu, Sep 07, 2017 at 10:29:34PM +0000, Tristram.Ha@microchip.com wrote:
> > > -----Original Message-----
> > > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > > Sent: Thursday, September 07, 2017 2:56 PM
> > > To: Tristram Ha - C24268
> > > Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> > > vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> > > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> > > Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
> > >
> > > On Thu, Sep 07, 2017 at 09:17:10PM +0000, Tristram.Ha@microchip.com wrote:
> > > > From: Tristram Ha <Tristram.Ha@microchip.com>
> > > >
> > > > Add KSZ8795 switch support with SPI access.
> > > >
> > > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > > ---
> > > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > > index 0961c30..0d8ba48 100644
> > > > --- a/drivers/net/dsa/microchip/Makefile
> > > > +++ b/drivers/net/dsa/microchip/Makefile
> > > > @@ -1,2 +1,4 @@
> > > > obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > > > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> > > > +obj-$(CONFIG_MICROCHIP_KSZ8795) += ksz8795.o ksz_common.o
> > > > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
> > >
> > > I've not tried it, but i think this breaks the build
> > >
> > > Andrew
> >
> > So you would like to have all 5 patches in 1 patch file?
>
> Or maybe this one last? Would that stop the build from breaking?
This and Kconfig change should come together.. and probably last.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* RE: [PATCH net] smsc95xx: Configure pause time to 0xffff when tx flow control enabled
From: Nisar.Sayed @ 2017-09-08 9:11 UTC (permalink / raw)
To: andrew; +Cc: davem, UNGLinuxDriver, netdev, steve.glendinning
In-Reply-To: <20170907135658.GG11248@lunn.ch>
> On Thu, Sep 07, 2017 at 06:51:37AM +0000, Nisar.Sayed@microchip.com
> wrote:
> > From: Nisar Sayed <Nisar.Sayed@microchip.com>
> >
> > Configure pause time to 0xffff when tx flow control enabled
>
> Hi Nisar
>
> You should explain the 'Why' in the commit message. Why do we want a
> pause time of 0xffff?
>
> Andrew
Thanks Andrew,
I shall update the description and submit next version.
- Nisar
^ permalink raw reply
* RE: [PATCH 3/3] lan78xx: Use default value loaded from EEPROM/OTP when resetting
From: Nisar.Sayed @ 2017-09-08 9:11 UTC (permalink / raw)
To: andrew; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <20170907141302.GK11248@lunn.ch>
> On Thu, Sep 07, 2017 at 07:11:50AM +0000, Nisar.Sayed@microchip.com
> wrote:
> > From: Nisar Sayed <Nisar.Sayed@microchip.com>
> >
> > Use default value loaded from EEPROM/OTP when resetting
>
> Hi Nisar
>
> Subject: [PATCH 3/3]
>
> Is this a fix for net, or further development for net-next?
>
> Why do we want the default values?
>
> Andrew
Thanks Andrew,
Yes it is for "net", sorry missed to include it, will update in next version of submit.
These bits are "reset protected" and should not be modified and must use the
Must be configured from EEPROM only. Will update the description.
- Nisar
^ permalink raw reply
* Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Pavel Machek @ 2017-09-08 9:18 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A87@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 8435 bytes --]
On Thu 2017-09-07 21:17:16, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add KSZ8795 switch support with function code.
English? "Add KSZ8795 switch support." ?
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> new file mode 100644
> index 0000000..e4d4e6a
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -0,0 +1,2066 @@
> +/*
> + * Microchip KSZ8795 switch driver
> + *
> + * Copyright (C) 2017 Microchip Technology Inc.
> + * Tristram Ha <Tristram.Ha@microchip.com>
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
This is not exactly GPL, right? But tagging below says it is
GPL. Please fix one.
> +/**
> + * Some counters do not need to be read too often because they are less likely
> + * to increase much.
> + */
Kerneldoc markup but this does not look like kerneldoc. Use plain /* instead?
> +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
> +{
> + u8 data;
> +
> + ksz_read8(dev, addr, &data);
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> + ksz_write8(dev, addr, data);
> +}
> +
> +static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
> + bool set)
> +{
> + u32 addr;
> + u8 data;
> +
> + addr = PORT_CTRL_ADDR(port, offset);
> + ksz_read8(dev, addr, &data);
> +
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> +
> + ksz_write8(dev, addr, data);
> +}
Other drivers will need this code, right? Does it make sense to put it
into header?
> +static int chk_last_port(struct ksz_device *dev, int p)
> +{
> + if (dev->last_port && p == dev->last_port)
> + p = dev->cpu_port;
> + return p;
> +}
We often prefix even static functions with common prefix. Up to you, I
guess.
> +static int ksz_reset_switch(struct ksz_device *dev)
> +{
> + /* reset switch */
> + ksz_write8(dev, REG_POWER_MANAGEMENT_1,
> + SW_SOFTWARE_POWER_DOWN << SW_POWER_MANAGEMENT_MODE_S);
> + ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
> +
> + return 0;
> +}
This is going to be same in other drivers, right?
> +
> + for (timeout = 1; timeout > 0; timeout--) {
> + ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> +
> + if (check & MIB_COUNTER_VALID) {
> + ksz_read32(dev, REG_IND_DATA_LO, &data);
> + if (addr < 2) {
> + u64 total;
> +
> + total = check & MIB_TOTAL_BYTES_H;
> + total <<= 32;
> + *cnt += total;
> + *cnt += data;
> + if (check & MIB_COUNTER_OVERFLOW) {
> + total = MIB_TOTAL_BYTES_H + 1;
> + total <<= 32;
> + *cnt += total;
> + }
> + } else {
> + if (check & MIB_COUNTER_OVERFLOW)
> + *cnt += MIB_PACKET_DROPPED + 1;
> + *cnt += data & MIB_PACKET_DROPPED;
> + }
> + break;
> + }
> + }
Why do you need a loop here? This is quite strange code. (And you have
similar strangeness elsewhere. Please fix.)
> +static int valid_dyn_entry(struct ksz_device *dev, u8 *data)
> +{
> + int timeout = 100;
> +
> + do {
> + ksz_read8(dev, REG_IND_DATA_CHECK, data);
> + timeout--;
> + } while ((*data & DYNAMIC_MAC_TABLE_NOT_READY) && timeout);
> +
> + /* Entry is not ready for accessing. */
> + if (*data & DYNAMIC_MAC_TABLE_NOT_READY) {
> + return 1;
> + /* Entry is ready for accessing. */
> + } else {
> + ksz_read8(dev, REG_IND_DATA_8, data);
> +
> + /* There is no valid entry in the table. */
> + if (*data & DYNAMIC_MAC_TABLE_MAC_EMPTY)
> + return 2;
> + }
> + return 0;
> +}
Normal calling convention is 0 / -ERROR, not 0,1,2.
> +static void ksz_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
> +{
> + struct ksz_port *port;
> + u8 ctrl;
> + u8 restart;
> + u8 link;
> + u8 speed;
> + u8 force;
> + u8 p = phy;
> + u16 data = 0;
> + int processed = true;
> +
> + port = &dev->ports[p];
> + switch (reg) {
> + case PHY_REG_CTRL:
> + ksz_pread8(dev, p, P_LOCAL_CTRL, &ctrl);
> + ksz_pread8(dev, p, P_NEG_RESTART_CTRL, &restart);
> + ksz_pread8(dev, p, P_SPEED_STATUS, &speed);
> + ksz_pread8(dev, p, P_FORCE_CTRL, &force);
> + if (restart & PORT_PHY_LOOPBACK)
> + data |= PHY_LOOPBACK;
> + if (force & PORT_FORCE_100_MBIT)
> + data |= PHY_SPEED_100MBIT;
> + if (!(force & PORT_AUTO_NEG_DISABLE))
> + data |= PHY_AUTO_NEG_ENABLE;
> + if (restart & PORT_POWER_DOWN)
> + data |= PHY_POWER_DOWN;
> + if (restart & PORT_AUTO_NEG_RESTART)
> + data |= PHY_AUTO_NEG_RESTART;
> + if (force & PORT_FORCE_FULL_DUPLEX)
> + data |= PHY_FULL_DUPLEX;
> + if (speed & PORT_HP_MDIX)
> + data |= PHY_HP_MDIX;
> + if (restart & PORT_FORCE_MDIX)
> + data |= PHY_FORCE_MDIX;
> + if (restart & PORT_AUTO_MDIX_DISABLE)
> + data |= PHY_AUTO_MDIX_DISABLE;
> + if (restart & PORT_TX_DISABLE)
> + data |= PHY_TRANSMIT_DISABLE;
> + if (restart & PORT_LED_OFF)
> + data |= PHY_LED_DISABLE;
> + break;
> + case PHY_REG_STATUS:
> + ksz_pread8(dev, p, P_LINK_STATUS, &link);
> + ksz_pread8(dev, p, P_SPEED_STATUS, &speed);
> + data = PHY_100BTX_FD_CAPABLE |
> + PHY_100BTX_CAPABLE |
> + PHY_10BT_FD_CAPABLE |
> + PHY_10BT_CAPABLE |
> + PHY_AUTO_NEG_CAPABLE;
> + if (link & PORT_AUTO_NEG_COMPLETE)
> + data |= PHY_AUTO_NEG_ACKNOWLEDGE;
> + if (link & PORT_STAT_LINK_GOOD) {
> + data |= PHY_LINK_STATUS;
> + if (!port->link_up) {
> + port->link_up = 1;
> + dev->live_ports |= (1 << phy) & dev->on_ports;
> + }
> + } else if (port->link_up) {
> + port->link_down = 1;
> + port->link_up = 0;
> + dev->live_ports &= ~(1 << phy);
> + }
> + break;
> + case PHY_REG_ID_1:
> + data = KSZ8795_ID_HI;
> + break;
> + case PHY_REG_ID_2:
> + data = KSZ8795_ID_LO;
> + break;
> + case PHY_REG_AUTO_NEGOTIATION:
> + ksz_pread8(dev, p, P_LOCAL_CTRL, &ctrl);
> + data = PHY_AUTO_NEG_802_3;
> + if (ctrl & PORT_AUTO_NEG_SYM_PAUSE)
> + data |= PHY_AUTO_NEG_SYM_PAUSE;
> + if (ctrl & PORT_AUTO_NEG_100BTX_FD)
> + data |= PHY_AUTO_NEG_100BTX_FD;
> + if (ctrl & PORT_AUTO_NEG_100BTX)
> + data |= PHY_AUTO_NEG_100BTX;
> + if (ctrl & PORT_AUTO_NEG_10BT_FD)
> + data |= PHY_AUTO_NEG_10BT_FD;
> + if (ctrl & PORT_AUTO_NEG_10BT)
> + data |= PHY_AUTO_NEG_10BT;
> + break;
> + case PHY_REG_REMOTE_CAPABILITY:
> + ksz_pread8(dev, p, P_REMOTE_STATUS, &link);
> + data = PHY_AUTO_NEG_802_3;
> + if (link & PORT_REMOTE_SYM_PAUSE)
> + data |= PHY_AUTO_NEG_SYM_PAUSE;
> + if (link & PORT_REMOTE_100BTX_FD)
> + data |= PHY_AUTO_NEG_100BTX_FD;
> + if (link & PORT_REMOTE_100BTX)
> + data |= PHY_AUTO_NEG_100BTX;
> + if (link & PORT_REMOTE_10BT_FD)
> + data |= PHY_AUTO_NEG_10BT_FD;
> + if (link & PORT_REMOTE_10BT)
> + data |= PHY_AUTO_NEG_10BT;
> + break;
> + default:
> + processed = false;
> + break;
> + }
> + if (processed)
> + *val = data;
> +}
Similar code will be needed by other drivers, right?
> +static int ksz_port_bridge_join(struct dsa_switch *ds, int port,
> + struct net_device *br)
> +{
> + struct ksz_device *dev = ds->priv;
> +
> + dev->br_member |= (1 << port);
> +
> + /**
> + * port_stp_state_set() will be called after to put the port in
> + * appropriate state so there is no need to do anything.
> + */
/** -> /*. Fix it elsewhere, too. This is not kerneldoc.
> + /* Topology is changed. */
> + if (forward != dev->member)
has changed?
Thanks for the patches,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 4/5] Add KSZ8795 register definitions
From: Pavel Machek @ 2017-09-08 9:21 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A8E@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]
On Thu 2017-09-07 21:17:27, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add KSZ8795 switch support with register definitions.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/ksz8795.h b/drivers/net/dsa/microchip/ksz8795.h
> new file mode 100644
> index 0000000..005eda5
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz8795.h
We already have ksz_9477_reg.h. Please make it consistent.
> @@ -0,0 +1,1015 @@
> +/**
> + * Microchip KSZ8795 definition file
> + *
> + * Copyright (c) 2017 Microchip Technology Inc.
> + * Tristram Ha <Tristram.Ha@microchip.com>
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
-ENOTGPL.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 5/5] Add KSZ8795 SPI driver
From: Pavel Machek @ 2017-09-08 9:26 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A95@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]
Hi!
> +static int ksz_spi_read(struct ksz_device *dev, u32 reg, u8 *data,
> + unsigned int len)
> +{
> + struct spi_device *spi = dev->priv;
> +
> + return ksz_spi_read_reg(spi, reg, data, len); }
> +
> +static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val) {
> + return ksz_spi_read(dev, reg, val, 1); }
> +
> +static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val) {
> + int ret = ksz_spi_read(dev, reg, (u8 *)val, 2);
> +
> + if (!ret)
> + *val = be16_to_cpu(*val);
> +
> + return ret;
> +}
> +static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val) {
> + int ret = ksz_spi_read(dev, reg, (u8 *)val, 4);
> +
> + if (!ret)
> + *val = be32_to_cpu(*val);
> +
> + return ret;
> +}
Please format according to CodingStyle. (Not only this.)
And this will be common for more drivers. Can it go to a header file
and be included...?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls of udelay to usleep_range
From: Pavel Machek @ 2017-09-08 9:28 UTC (permalink / raw)
To: Brown, Aaron F
Cc: Matthew Tan, intel-wired-lan@lists.osuosl.org,
michael.kardonik@nxp.com, Williams, Mitch A,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B8C69E5E5@ORSMSX101.amr.corp.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]
On Thu 2017-09-07 22:19:47, Brown, Aaron F wrote:
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> > Behalf Of Pavel Machek
> > Sent: Monday, September 4, 2017 9:26 AM
> > To: Matthew Tan <matthew.tan_1@nxp.com>
> > Cc: michael.kardonik@nxp.com; Williams, Mitch A
> > <mitch.a.williams@intel.com>; linux-kernel@vger.kernel.org;
> > john.ronciak@intel.com; intel-wired-lan@lists.osuosl.org;
> > netdev@vger.kernel.org
> > Subject: Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls
> > of udelay to usleep_range
> >
> > Hi!
> >
> > > @@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw
> > *hw, u32 offset, u16 *data)
> > > * reading duplicate data in the next MDIC transaction.
> > > */
> > > if (hw->mac.type == e1000_pch2lan)
> > > - udelay(100);
> > > + usleep_range(90, 100);
> > >
> > > return 0;
> > > }
> >
> > Can you explain why shortening the delay is acceptable here?
>
> Maybe it's not.
>
> This patch is causing speed / duplex tests to fail on several of my test systems. Specifically a Lenova laptop with an 82577 and a NUC with an i218 (though that does not mean it is limited to those or that it's not related to the individual link partner.)
>
Ok, this should be quite easy to verify -- just adjust all the ranges
to be >= original ones.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-08 10:10 UTC (permalink / raw)
To: Kosuke Tatsukawa
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <17EC94B0A072C34B8DCF0D30AD16044A029864AC@BPXM09GP.gisp.nec.co.jp>
On 08/09/17 05:06, Kosuke Tatsukawa wrote:
> Hi,
>
>> On 7.09.2017 01:47, Kosuke Tatsukawa wrote:
>>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>>> balance-alb mode") tried to fix transmit dynamic load balancing in
>>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>>> ("bonding: remove hardcoded value").
>>>
>>> It turned out that my previous patch only fixed the case when
>>> balance-alb was specified as bonding module parameter, and not when
>>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>>> common usage). In the latter case, tlb_dynamic_lb was set up according
>>> to the default mode of the bonding interface, which happens to be
>>> balance-rr.
>>>
>>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>>
>>> I didn't add code to change tlb_balance_lb back to the default value for
>>> other modes, because "mode" is usually set up only once during
>>> initialization, and it's not worthwhile to change the static variable
>>> bonding_defaults in bond_main.c to a global variable just for this
>>> purpose.
>>>
>>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>>> balance-tlb mode if it is set up using the sysfs interface. I didn't
>>> change that behavior, because the value of tlb_balance_lb can be changed
>>> using the sysfs interface for balance-tlb, and I didn't like changing
>>> the default value back and forth for balance-tlb.
>>>
>>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>>> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
>>> is not an intended usage, so there is little use making it writable at
>>> this moment.
>>>
>>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>>> Reported-by: Reinis Rozitis <r@roze.lv>
>>> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
>>> Cc: stable@vger.kernel.org # v4.12+
>>> ---
>>> drivers/net/bonding/bond_options.c | 3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>
>> I don't believe this to be the right solution, hardcoding it like this
>> changes user-visible behaviour. The issue is that if someone configured
>> it to be 0 in tlb mode, suddenly it will become 1 and will silently
>> override their config if they switch the mode to alb. Also it robs users
>> from their choice.
>>
>> If you think this should be settable in ALB mode, then IMO you should
>> edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
>> That would also be consistent with how it's handled in TLB mode.
>
> No, I don't think tlb_dynamic_lb should be settable in balance-alb at
> this point. All the current commits regarding tlb_dynamic_lb are for
> balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
> to 0 is an intended usage.
>
>
>> Going back and looking at your previous fix I'd argue that it is also
>> wrong, you should've removed the mode check altogether to return the
>> original behaviour where the dynamic_lb is set to 1 (enabled) by
>> default and then ALB mode would've had it, of course that would've left
>> the case of setting it to 0 in TLB mode and switching to ALB, but that
>> is a different issue.
>
> Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
> tlb_dynamic_lb is referenced in the following functions.
>
> + bond_do_alb_xmit() -- Used by both balance-tlb and balance-alb
> + bond_tlb_xmit() -- Only used by balance-tlb
> + bond_open() -- Used by both balance-tlb and balance-alb
> + bond_check_params() -- Used during module initialization
> + bond_fill_info() -- Used to get/set value
> + bond_option_tlb_dynamic_lb_set() -- Used to get/set value
> + bonding_show_tlb_dynamic_lb() -- Used to get/set value
> + bond_is_nondyn_tlb() -- Only referenced if balance-tlb mode
>
> The following untested patch adds code to make balance-alb work as if
> tlb_dynamic_lb==1 for the functions which affect balance-alb mode. It
> also reverts my previous patch.
>
> What do you think about this approach?
> ---
> Kosuke TATSUKAWA | 1st Platform Software Division
> | NEC Solution Innovators
> | tatsu@ab.jp.nec.com
>
Logically the approach looks good, that being said it adds unnecessary tests in
the fast path, why not just something like the patch below ? That only leaves the
problem if it is zeroed in TLB and switched to ALB mode, and that is a one line
fix to unsuppmodes just allow it to be set for that specific case. The below
returns the default behaviour before the commit in your Fixes tag.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fc63992ab0e0..c99dc59d729b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4289,7 +4289,7 @@ static int bond_check_params(struct bond_params *params)
int bond_mode = BOND_MODE_ROUNDROBIN;
int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
int lacp_fast = 0;
- int tlb_dynamic_lb = 0;
+ int tlb_dynamic_lb;
/* Convert string parameters. */
if (mode) {
@@ -4601,16 +4601,13 @@ static int bond_check_params(struct bond_params *params)
}
ad_user_port_key = valptr->value;
- if ((bond_mode == BOND_MODE_TLB) || (bond_mode == BOND_MODE_ALB)) {
- bond_opt_initstr(&newval, "default");
- valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB),
- &newval);
- if (!valptr) {
- pr_err("Error: No tlb_dynamic_lb default value");
- return -EINVAL;
- }
- tlb_dynamic_lb = valptr->value;
+ bond_opt_initstr(&newval, "default");
+ valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB), &newval);
+ if (!valptr) {
+ pr_err("Error: No tlb_dynamic_lb default value");
+ return -EINVAL;
}
+ tlb_dynamic_lb = valptr->value;
if (lp_interval == 0) {
pr_warn("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
^ permalink raw reply related
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-08 10:13 UTC (permalink / raw)
To: Kosuke Tatsukawa
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <52a22af7-cc99-e441-79fa-e48da283b001@cumulusnetworks.com>
On 08/09/17 13:10, Nikolay Aleksandrov wrote:
> On 08/09/17 05:06, Kosuke Tatsukawa wrote:
>> Hi,
>>
>>> On 7.09.2017 01:47, Kosuke Tatsukawa wrote:
>>>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>>>> balance-alb mode") tried to fix transmit dynamic load balancing in
>>>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>>>> ("bonding: remove hardcoded value").
>>>>
>>>> It turned out that my previous patch only fixed the case when
>>>> balance-alb was specified as bonding module parameter, and not when
>>>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>>>> common usage). In the latter case, tlb_dynamic_lb was set up according
>>>> to the default mode of the bonding interface, which happens to be
>>>> balance-rr.
>>>>
>>>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>>>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>>>
>>>> I didn't add code to change tlb_balance_lb back to the default value for
>>>> other modes, because "mode" is usually set up only once during
>>>> initialization, and it's not worthwhile to change the static variable
>>>> bonding_defaults in bond_main.c to a global variable just for this
>>>> purpose.
>>>>
>>>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>>>> balance-tlb mode if it is set up using the sysfs interface. I didn't
>>>> change that behavior, because the value of tlb_balance_lb can be changed
>>>> using the sysfs interface for balance-tlb, and I didn't like changing
>>>> the default value back and forth for balance-tlb.
>>>>
>>>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>>>> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
>>>> is not an intended usage, so there is little use making it writable at
>>>> this moment.
>>>>
>>>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>>>> Reported-by: Reinis Rozitis <r@roze.lv>
>>>> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
>>>> Cc: stable@vger.kernel.org # v4.12+
>>>> ---
>>>> drivers/net/bonding/bond_options.c | 3 +++
>>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>
>>> I don't believe this to be the right solution, hardcoding it like this
>>> changes user-visible behaviour. The issue is that if someone configured
>>> it to be 0 in tlb mode, suddenly it will become 1 and will silently
>>> override their config if they switch the mode to alb. Also it robs users
>>> from their choice.
>>>
>>> If you think this should be settable in ALB mode, then IMO you should
>>> edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
>>> That would also be consistent with how it's handled in TLB mode.
>>
>> No, I don't think tlb_dynamic_lb should be settable in balance-alb at
>> this point. All the current commits regarding tlb_dynamic_lb are for
>> balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
>> to 0 is an intended usage.
>>
>>
>>> Going back and looking at your previous fix I'd argue that it is also
>>> wrong, you should've removed the mode check altogether to return the
>>> original behaviour where the dynamic_lb is set to 1 (enabled) by
>>> default and then ALB mode would've had it, of course that would've left
>>> the case of setting it to 0 in TLB mode and switching to ALB, but that
>>> is a different issue.
>>
>> Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
>> tlb_dynamic_lb is referenced in the following functions.
>>
>> + bond_do_alb_xmit() -- Used by both balance-tlb and balance-alb
>> + bond_tlb_xmit() -- Only used by balance-tlb
>> + bond_open() -- Used by both balance-tlb and balance-alb
>> + bond_check_params() -- Used during module initialization
>> + bond_fill_info() -- Used to get/set value
>> + bond_option_tlb_dynamic_lb_set() -- Used to get/set value
>> + bonding_show_tlb_dynamic_lb() -- Used to get/set value
>> + bond_is_nondyn_tlb() -- Only referenced if balance-tlb mode
>>
>> The following untested patch adds code to make balance-alb work as if
>> tlb_dynamic_lb==1 for the functions which affect balance-alb mode. It
>> also reverts my previous patch.
>>
>> What do you think about this approach?
>> ---
>> Kosuke TATSUKAWA | 1st Platform Software Division
>> | NEC Solution Innovators
>> | tatsu@ab.jp.nec.com
>>
>
> Logically the approach looks good, that being said it adds unnecessary tests in
> the fast path, why not just something like the patch below ? That only leaves the
> problem if it is zeroed in TLB and switched to ALB mode, and that is a one line
> fix to unsuppmodes just allow it to be set for that specific case. The below
> returns the default behaviour before the commit in your Fixes tag.
>
>
Actually I'm fine with your approach, too. It will fix this regardless of the
value of tlb_dynamic_lb which sounds good to me for the price of a test in
the fast path.
^ permalink raw reply
* [PATCH iproute2 0/2] malloc correct buff at run time
From: Hangbin Liu @ 2017-09-08 10:14 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Michal Kubecek, Phil Sutter, Hangbin Liu
With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size") and
460c03f3f3cc ("iplink: double the buffer size also in iplink_get()"), we
extend the buffer size to avoid truncated message with large numbers of
VFs. But just as Michal said, this is not future-proof since the NIC
number is increasing. We have customer even has 220+ VFs now.
This is not make sense to hard code the buffer and increase it all the time.
So let's just malloc the correct buff size at run time.
I'm not sure what init size would be suitable, so I keep use the original
size. I have tried with a small size like 1024, and it also works.
I tested with most ip cmds and all looks good.
Hangbin Liu (2):
lib/libnetlink: re malloc buff if size is not enough
lib/libnetlink: update rtnl_talk to support malloc buff at run time
bridge/fdb.c | 2 +-
bridge/link.c | 2 +-
bridge/mdb.c | 2 +-
bridge/vlan.c | 2 +-
genl/ctrl.c | 14 +++---
include/libnetlink.h | 6 +--
ip/ipaddress.c | 5 ++-
ip/ipaddrlabel.c | 4 +-
ip/ipfou.c | 4 +-
ip/ipila.c | 4 +-
ip/ipl2tp.c | 8 ++--
ip/iplink.c | 28 +++++-------
ip/iplink_vrf.c | 24 ++++-------
ip/ipmacsec.c | 2 +-
ip/ipneigh.c | 2 +-
ip/ipnetns.c | 13 +++---
ip/ipntable.c | 2 +-
ip/iproute.c | 20 +++++----
ip/iprule.c | 7 +--
ip/ipseg6.c | 7 +--
ip/iptoken.c | 2 +-
ip/link_gre.c | 7 +--
ip/link_gre6.c | 7 +--
ip/link_ip6tnl.c | 7 +--
ip/link_iptnl.c | 7 +--
ip/link_vti.c | 7 +--
ip/link_vti6.c | 7 +--
ip/tcp_metrics.c | 7 +--
ip/xfrm_policy.c | 22 +++++-----
ip/xfrm_state.c | 25 ++++++-----
lib/libgenl.c | 5 ++-
lib/libnetlink.c | 118 ++++++++++++++++++++++++++++++++-------------------
misc/ss.c | 2 +-
tc/m_action.c | 9 ++--
tc/tc_class.c | 2 +-
tc/tc_filter.c | 7 +--
tc/tc_qdisc.c | 2 +-
37 files changed, 217 insertions(+), 184 deletions(-)
--
2.5.5
^ permalink raw reply
* [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Hangbin Liu @ 2017-09-08 10:14 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Michal Kubecek, Phil Sutter, Hangbin Liu
In-Reply-To: <1504865697-27274-1-git-send-email-liuhangbin@gmail.com>
With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size")
we doubled the buffer size to support more VFs. But the VFs number is
increasing all the time. Some customers even use more than 200 VFs now.
We could not double it everytime when the buffer is not enough. Let's just
not hard code the buffer size and malloc the correct number when running.
Introduce a new function rtnl_recvmsg() to get correct buffer.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
lib/libnetlink.c | 98 ++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 66 insertions(+), 32 deletions(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index be7ac86..37cfb5a 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle *rth,
}
}
+static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
+{
+ struct iovec *iov;
+ int len = -1, buf_len = 32768;
+ char *buffer = *buf;
+
+ int flag = MSG_PEEK | MSG_TRUNC;
+
+ if (buffer == NULL)
+re_malloc:
+ buffer = malloc(buf_len);
+
+ if (buffer == NULL) {
+ fprintf(stderr, "malloc error: no enough buffer\n");
+ return -1;
+ }
+
+ iov = msg->msg_iov;
+ iov->iov_base = buffer;
+ iov->iov_len = buf_len;
+
+re_recv:
+ len = recvmsg(fd, msg, flag);
+
+ if (len < 0) {
+ if (errno == EINTR || errno == EAGAIN)
+ return 0;
+ fprintf(stderr, "netlink receive error %s (%d)\n",
+ strerror(errno), errno);
+ return len;
+ }
+
+ if (len == 0) {
+ fprintf(stderr, "EOF on netlink\n");
+ return -1;
+ }
+
+ if (len > buf_len) {
+ free(buffer);
+ buf_len = len;
+ flag = 0;
+ goto re_malloc;
+ }
+
+ if (flag != 0) {
+ flag = 0;
+ goto re_recv;
+ }
+
+ *buf = buffer;
+ return len;
+}
+
int rtnl_dump_filter_l(struct rtnl_handle *rth,
const struct rtnl_dump_filter_arg *arg)
{
@@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
.msg_iov = &iov,
.msg_iovlen = 1,
};
- char buf[32768];
+ static char *buf = NULL;
int dump_intr = 0;
- iov.iov_base = buf;
while (1) {
int status;
const struct rtnl_dump_filter_arg *a;
int found_done = 0;
int msglen = 0;
- iov.iov_len = sizeof(buf);
- status = recvmsg(rth->fd, &msg, 0);
-
- if (status < 0) {
- if (errno == EINTR || errno == EAGAIN)
- continue;
- fprintf(stderr, "netlink receive error %s (%d)\n",
- strerror(errno), errno);
- return -1;
- }
-
- if (status == 0) {
- fprintf(stderr, "EOF on netlink\n");
- return -1;
- }
+ status = rtnl_recvmsg(rth->fd, &msg, &buf);
+ if (status < 0)
+ return status;
+ else if (status == 0)
+ continue;
if (rth->dump_fp)
fwrite(buf, 1, NLMSG_ALIGN(status), rth->dump_fp);
@@ -543,7 +585,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
.msg_iov = &iov,
.msg_iovlen = 1,
};
- char buf[32768] = {};
+ static char *buf = NULL;
n->nlmsg_seq = seq = ++rtnl->seq;
@@ -556,22 +598,14 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
return -1;
}
- iov.iov_base = buf;
while (1) {
- iov.iov_len = sizeof(buf);
- status = recvmsg(rtnl->fd, &msg, 0);
+ status = rtnl_recvmsg(rtnl->fd, &msg, &buf);
+
+ if (status < 0)
+ return status;
+ else if (status == 0)
+ continue;
- if (status < 0) {
- if (errno == EINTR || errno == EAGAIN)
- continue;
- fprintf(stderr, "netlink receive error %s (%d)\n",
- strerror(errno), errno);
- return -1;
- }
- if (status == 0) {
- fprintf(stderr, "EOF on netlink\n");
- return -1;
- }
if (msg.msg_namelen != sizeof(nladdr)) {
fprintf(stderr,
"sender address length == %d\n",
--
2.5.5
^ permalink raw reply related
* [PATCH iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time
From: Hangbin Liu @ 2017-09-08 10:14 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Michal Kubecek, Phil Sutter, Hangbin Liu
In-Reply-To: <1504865697-27274-1-git-send-email-liuhangbin@gmail.com>
This is an update for 460c03f3f3cc ("iplink: double the buffer size also in
iplink_get()"). After update, we will not need to double the buffer size
every time when VFs number increased.
With call like rtnl_talk(&rth, &req.n, NULL, 0), we can simply remove the
length parameter.
With call like rtnl_talk(&rth, nlh, nlh, sizeof(req), I add a new variable
answer to avoid overwrite data in nlh, because it may has more info after
nlh. also this will avoid nlh buffer not enough issue.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
bridge/fdb.c | 2 +-
bridge/link.c | 2 +-
bridge/mdb.c | 2 +-
bridge/vlan.c | 2 +-
genl/ctrl.c | 14 ++++++++------
include/libnetlink.h | 6 +++---
ip/ipaddress.c | 5 +++--
ip/ipaddrlabel.c | 4 ++--
ip/ipfou.c | 4 ++--
ip/ipila.c | 4 ++--
ip/ipl2tp.c | 8 ++++----
ip/iplink.c | 28 +++++++++++-----------------
ip/iplink_vrf.c | 24 ++++++++----------------
ip/ipmacsec.c | 2 +-
ip/ipneigh.c | 2 +-
ip/ipnetns.c | 13 +++++++------
ip/ipntable.c | 2 +-
ip/iproute.c | 20 +++++++++++---------
ip/iprule.c | 7 ++++---
ip/ipseg6.c | 7 ++++---
ip/iptoken.c | 2 +-
ip/link_gre.c | 7 ++++---
ip/link_gre6.c | 7 ++++---
ip/link_ip6tnl.c | 7 ++++---
ip/link_iptnl.c | 7 ++++---
ip/link_vti.c | 7 ++++---
ip/link_vti6.c | 7 ++++---
ip/tcp_metrics.c | 7 ++++---
ip/xfrm_policy.c | 22 +++++++++++-----------
ip/xfrm_state.c | 25 ++++++++++++-------------
lib/libgenl.c | 5 +++--
lib/libnetlink.c | 20 +++++++++-----------
misc/ss.c | 2 +-
tc/m_action.c | 9 ++++-----
tc/tc_class.c | 2 +-
tc/tc_filter.c | 7 ++++---
tc/tc_qdisc.c | 2 +-
37 files changed, 151 insertions(+), 152 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index e5cebf9..807914f 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -535,7 +535,7 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
return -1;
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -1;
return 0;
diff --git a/bridge/link.c b/bridge/link.c
index 93472ad..cc29a2a 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -426,7 +426,7 @@ static int brlink_modify(int argc, char **argv)
addattr_nest_end(&req.n, nest);
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -1;
return 0;
diff --git a/bridge/mdb.c b/bridge/mdb.c
index e60ff3e..fbd8184 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -298,7 +298,7 @@ static int mdb_modify(int cmd, int flags, int argc, char **argv)
entry.vid = vid;
addattr_l(&req.n, sizeof(req), MDBA_SET_ENTRY, &entry, sizeof(entry));
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -1;
return 0;
diff --git a/bridge/vlan.c b/bridge/vlan.c
index ebcdace..5d68359 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -133,7 +133,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
addattr_nest_end(&req.n, afspec);
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -1;
return 0;
diff --git a/genl/ctrl.c b/genl/ctrl.c
index 448988e..699657b 100644
--- a/genl/ctrl.c
+++ b/genl/ctrl.c
@@ -55,6 +55,7 @@ int genl_ctrl_resolve_family(const char *family)
};
struct nlmsghdr *nlh = &req.n;
struct genlmsghdr *ghdr = &req.g;
+ struct nlmsghdr *answer = NULL;
if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC) < 0) {
fprintf(stderr, "Cannot open generic netlink socket\n");
@@ -63,19 +64,19 @@ int genl_ctrl_resolve_family(const char *family)
addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, family, strlen(family) + 1);
- if (rtnl_talk(&rth, nlh, nlh, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, nlh, &answer) < 0) {
fprintf(stderr, "Error talking to the kernel\n");
goto errout;
}
{
struct rtattr *tb[CTRL_ATTR_MAX + 1];
- int len = nlh->nlmsg_len;
+ int len = answer->nlmsg_len;
struct rtattr *attrs;
- if (nlh->nlmsg_type != GENL_ID_CTRL) {
+ if (answer->nlmsg_type != GENL_ID_CTRL) {
fprintf(stderr, "Not a controller message, nlmsg_len=%d "
- "nlmsg_type=0x%x\n", nlh->nlmsg_len, nlh->nlmsg_type);
+ "nlmsg_type=0x%x\n", answer->nlmsg_len, answer->nlmsg_type);
goto errout;
}
@@ -299,6 +300,7 @@ static int ctrl_list(int cmd, int argc, char **argv)
.g.cmd = CTRL_CMD_GETFAMILY,
};
struct nlmsghdr *nlh = &req.n;
+ struct nlmsghdr *answer = NULL;
if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC) < 0) {
fprintf(stderr, "Cannot open generic netlink socket\n");
@@ -331,12 +333,12 @@ static int ctrl_list(int cmd, int argc, char **argv)
goto ctrl_done;
}
- if (rtnl_talk(&rth, nlh, nlh, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, nlh, &answer) < 0) {
fprintf(stderr, "Error talking to the kernel\n");
goto ctrl_done;
}
- if (print_ctrl2(NULL, nlh, (void *) stdout) < 0) {
+ if (print_ctrl2(NULL, answer, (void *) stdout) < 0) {
fprintf(stderr, "Dump terminated\n");
goto ctrl_done;
}
diff --git a/include/libnetlink.h b/include/libnetlink.h
index 69257f0..77b6260 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -93,13 +93,13 @@ int rtnl_dump_filter_nc(struct rtnl_handle *rth,
#define rtnl_dump_filter(rth, filter, arg) \
rtnl_dump_filter_nc(rth, filter, arg, 0)
int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
- struct nlmsghdr *answer, size_t len)
+ struct nlmsghdr **answer)
__attribute__((warn_unused_result));
int rtnl_talk_extack(struct rtnl_handle *rtnl, struct nlmsghdr *n,
- struct nlmsghdr *answer, size_t len, nl_ext_ack_fn_t errfn)
+ struct nlmsghdr **answer, nl_ext_ack_fn_t errfn)
__attribute__((warn_unused_result));
int rtnl_talk_suppress_rtnl_errmsg(struct rtnl_handle *rtnl, struct nlmsghdr *n,
- struct nlmsghdr *answer, size_t len)
+ struct nlmsghdr **answer)
__attribute__((warn_unused_result));
int rtnl_send(struct rtnl_handle *rth, const void *buf, int)
__attribute__((warn_unused_result));
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index c9312f0..f84e591 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1385,12 +1385,13 @@ static int restore_handler(const struct sockaddr_nl *nl,
struct nlmsghdr *n, void *arg)
{
int ret;
+ struct nlmsghdr *answer = NULL;
n->nlmsg_flags |= NLM_F_REQUEST | NLM_F_CREATE | NLM_F_ACK;
ll_init_map(&rth);
- ret = rtnl_talk(&rth, n, n, sizeof(*n));
+ ret = rtnl_talk(&rth, n, &answer);
if ((ret < 0) && (errno == EEXIST))
ret = 0;
@@ -2076,7 +2077,7 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
return -1;
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -2;
return 0;
diff --git a/ip/ipaddrlabel.c b/ip/ipaddrlabel.c
index 1d324da..6ea9bff 100644
--- a/ip/ipaddrlabel.c
+++ b/ip/ipaddrlabel.c
@@ -176,7 +176,7 @@ static int ipaddrlabel_modify(int cmd, int argc, char **argv)
if (req.ifal.ifal_family == AF_UNSPEC)
req.ifal.ifal_family = AF_INET6;
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -203,7 +203,7 @@ static int flush_addrlabel(const struct sockaddr_nl *who, struct nlmsghdr *n, vo
if (rtnl_open(&rth2, 0) < 0)
return -1;
- if (rtnl_talk(&rth2, n, NULL, 0) < 0)
+ if (rtnl_talk(&rth2, n, NULL) < 0)
return -2;
rtnl_close(&rth2);
diff --git a/ip/ipfou.c b/ip/ipfou.c
index 00dbe15..23000dc 100644
--- a/ip/ipfou.c
+++ b/ip/ipfou.c
@@ -116,7 +116,7 @@ static int do_add(int argc, char **argv)
fou_parse_opt(argc, argv, &req.n, true);
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -128,7 +128,7 @@ static int do_del(int argc, char **argv)
fou_parse_opt(argc, argv, &req.n, false);
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
diff --git a/ip/ipila.c b/ip/ipila.c
index 843cc16..0403fc4 100644
--- a/ip/ipila.c
+++ b/ip/ipila.c
@@ -220,7 +220,7 @@ static int do_add(int argc, char **argv)
ila_parse_opt(argc, argv, &req.n, true);
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -232,7 +232,7 @@ static int do_del(int argc, char **argv)
ila_parse_opt(argc, argv, &req.n, false);
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index 88664c9..742adbe 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -129,7 +129,7 @@ static int create_tunnel(struct l2tp_parm *p)
addattr(&req.n, 1024, L2TP_ATTR_UDP_ZERO_CSUM6_RX);
}
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -142,7 +142,7 @@ static int delete_tunnel(struct l2tp_parm *p)
addattr32(&req.n, 128, L2TP_ATTR_CONN_ID, p->tunnel_id);
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -185,7 +185,7 @@ static int create_session(struct l2tp_parm *p)
if (p->ifname && p->ifname[0])
addattrstrz(&req.n, 1024, L2TP_ATTR_IFNAME, p->ifname);
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -198,7 +198,7 @@ static int delete_session(struct l2tp_parm *p)
addattr32(&req.n, 1024, L2TP_ATTR_CONN_ID, p->tunnel_id);
addattr32(&req.n, 1024, L2TP_ATTR_SESSION_ID, p->session_id);
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
diff --git a/ip/iplink.c b/ip/iplink.c
index 72c3479..e078d2a 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -248,16 +248,18 @@ static int nl_get_ll_addr_len(unsigned int dev_index)
.ifi_index = dev_index,
}
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX+1];
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
return -1;
- len = req.n.nlmsg_len - NLMSG_LENGTH(sizeof(req.i));
+ len = answer->nlmsg_len - NLMSG_LENGTH(sizeof(struct ifinfomsg));
if (len < 0)
return -1;
- parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(&req.i), len, NLA_F_NESTED);
+ parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(NLMSG_DATA(answer)),
+ len, NLA_F_NESTED);
if (!tb[IFLA_ADDRESS])
return -1;
@@ -912,7 +914,7 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
req.i.ifi_index = 0;
addattr32(&req.n, sizeof(req), IFLA_GROUP, group);
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -2;
return 0;
}
@@ -1007,7 +1009,7 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
return -1;
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -1022,10 +1024,7 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
.n.nlmsg_type = RTM_GETLINK,
.i.ifi_family = preferred_family,
};
- struct {
- struct nlmsghdr n;
- char buf[32768];
- } answer;
+ struct nlmsghdr *answer = NULL;
if (name) {
len = strlen(name) + 1;
@@ -1038,18 +1037,13 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
}
addattr32(&req.n, sizeof(req), IFLA_EXT_MASK, filt_mask);
- if (rtnl_talk(&rth, &req.n, &answer.n, sizeof(answer)) < 0)
- return -2;
- if (answer.n.nlmsg_len > sizeof(answer.buf)) {
- fprintf(stderr, "Message truncated from %u to %lu\n",
- answer.n.nlmsg_len, sizeof(answer.buf));
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
return -2;
- }
if (brief)
- print_linkinfo_brief(NULL, &answer.n, stdout, NULL);
+ print_linkinfo_brief(NULL, answer, stdout, NULL);
else
- print_linkinfo(NULL, &answer.n, stdout);
+ print_linkinfo(NULL, answer, stdout);
return 0;
}
diff --git a/ip/iplink_vrf.c b/ip/iplink_vrf.c
index 2b85a3a..6dbc8b4 100644
--- a/ip/iplink_vrf.c
+++ b/ip/iplink_vrf.c
@@ -114,10 +114,7 @@ __u32 ipvrf_get_table(const char *name)
.ifi_family = preferred_family,
},
};
- struct {
- struct nlmsghdr n;
- char buf[8192];
- } answer;
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX+1];
struct rtattr *li[IFLA_INFO_MAX+1];
struct rtattr *vrf_attr[IFLA_VRF_MAX + 1];
@@ -127,8 +124,7 @@ __u32 ipvrf_get_table(const char *name)
addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name, strlen(name) + 1);
- if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n,
- &answer.n, sizeof(answer)) < 0) {
+ if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0) {
/* special case "default" vrf to be the main table */
if (errno == ENODEV && !strcmp(name, "default"))
if (rtnl_rttable_a2n(&tb_id, "main"))
@@ -138,8 +134,8 @@ __u32 ipvrf_get_table(const char *name)
return tb_id;
}
- ifi = NLMSG_DATA(&answer.n);
- len = answer.n.nlmsg_len - NLMSG_LENGTH(sizeof(*ifi));
+ ifi = NLMSG_DATA(answer);
+ len = answer->nlmsg_len - NLMSG_LENGTH(sizeof(*ifi));
if (len < 0) {
fprintf(stderr, "BUG: Invalid response to link query.\n");
return 0;
@@ -184,10 +180,7 @@ int name_is_vrf(const char *name)
.ifi_family = preferred_family,
},
};
- struct {
- struct nlmsghdr n;
- char buf[8192];
- } answer;
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX+1];
struct rtattr *li[IFLA_INFO_MAX+1];
struct ifinfomsg *ifi;
@@ -195,12 +188,11 @@ int name_is_vrf(const char *name)
addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name, strlen(name) + 1);
- if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n,
- &answer.n, sizeof(answer)) < 0)
+ if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0)
return 0;
- ifi = NLMSG_DATA(&answer.n);
- len = answer.n.nlmsg_len - NLMSG_LENGTH(sizeof(*ifi));
+ ifi = NLMSG_DATA(answer);
+ len = answer->nlmsg_len - NLMSG_LENGTH(sizeof(*ifi));
if (len < 0) {
fprintf(stderr, "BUG: Invalid response to link query.\n");
return 0;
diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index aa89a00..9a2d0eb 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -421,7 +421,7 @@ static int do_modify_nl(enum cmd c, enum macsec_nl_commands cmd, int ifindex,
addattr_nest_end(&req.n, attr_sa);
talk:
- if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
return -2;
return 0;
diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 9c38a60..32f2d55 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -184,7 +184,7 @@ static int ipneigh_modify(int cmd, int flags, int argc, char **argv)
return -1;
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
return 0;
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index afb4978..628dfb1 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -95,12 +95,13 @@ static int get_netnsid_from_name(const char *name)
struct nlmsghdr n;
struct rtgenmsg g;
char buf[1024];
- } answer, req = {
+ } req = {
.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg)),
.n.nlmsg_flags = NLM_F_REQUEST,
.n.nlmsg_type = RTM_GETNSID,
.g.rtgen_family = AF_UNSPEC,
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[NETNSA_MAX + 1];
struct rtgenmsg *rthdr;
int len, fd;
@@ -110,18 +111,18 @@ static int get_netnsid_from_name(const char *name)
return fd;
addattr32(&req.n, 1024, NETNSA_FD, fd);
- if (rtnl_talk(&rtnsh, &req.n, &answer.n, sizeof(answer)) < 0) {
+ if (rtnl_talk(&rtnsh, &req.n, &answer) < 0) {
close(fd);
return -2;
}
close(fd);
/* Validate message and parse attributes */
- if (answer.n.nlmsg_type == NLMSG_ERROR)
+ if (answer->nlmsg_type == NLMSG_ERROR)
return -1;
- rthdr = NLMSG_DATA(&answer.n);
- len = answer.n.nlmsg_len - NLMSG_SPACE(sizeof(*rthdr));
+ rthdr = NLMSG_DATA(answer);
+ len = answer->nlmsg_len - NLMSG_SPACE(sizeof(*rthdr));
if (len < 0)
return -1;
@@ -689,7 +690,7 @@ static int set_netnsid_from_name(const char *name, int nsid)
addattr32(&req.n, 1024, NETNSA_FD, fd);
addattr32(&req.n, 1024, NETNSA_NSID, nsid);
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
err = -2;
close(fd);
diff --git a/ip/ipntable.c b/ip/ipntable.c
index 88236ce..2f72c98 100644
--- a/ip/ipntable.c
+++ b/ip/ipntable.c
@@ -304,7 +304,7 @@ static int ipntable_modify(int cmd, int flags, int argc, char **argv)
RTA_PAYLOAD(parms_rta));
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
return 0;
diff --git a/ip/iproute.c b/ip/iproute.c
index 83fd70c..aa929d9 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1298,7 +1298,7 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
if (!type_ok && req.r.rtm_family == AF_MPLS)
req.r.rtm_type = RTN_UNICAST;
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -2;
return 0;
@@ -1678,6 +1678,7 @@ static int iproute_get(int argc, char **argv)
};
char *idev = NULL;
char *odev = NULL;
+ struct nlmsghdr *answer = NULL;
int connected = 0;
int fib_match = 0;
int from_ok = 0;
@@ -1798,20 +1799,20 @@ static int iproute_get(int argc, char **argv)
if (fib_match)
req.r.rtm_flags |= RTM_F_FIB_MATCH;
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
return -2;
if (connected && !from_ok) {
- struct rtmsg *r = NLMSG_DATA(&req.n);
- int len = req.n.nlmsg_len;
+ struct rtmsg *r = NLMSG_DATA(answer);
+ int len = answer->nlmsg_len;
struct rtattr *tb[RTA_MAX+1];
- if (print_route(NULL, &req.n, (void *)stdout) < 0) {
+ if (print_route(NULL, answer, (void *)stdout) < 0) {
fprintf(stderr, "An error :-)\n");
return -1;
}
- if (req.n.nlmsg_type != RTM_NEWROUTE) {
+ if (answer->nlmsg_type != RTM_NEWROUTE) {
fprintf(stderr, "Not a route?\n");
return -1;
}
@@ -1841,11 +1842,11 @@ static int iproute_get(int argc, char **argv)
req.n.nlmsg_flags = NLM_F_REQUEST;
req.n.nlmsg_type = RTM_GETROUTE;
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
return -2;
}
- if (print_route(NULL, &req.n, (void *)stdout) < 0) {
+ if (print_route(NULL, answer, (void *)stdout) < 0) {
fprintf(stderr, "An error :-)\n");
return -1;
}
@@ -1869,6 +1870,7 @@ static int restore_handler(const struct sockaddr_nl *nl,
struct rtattr *tb[RTA_MAX+1];
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
int ret, prio = *(int *)arg;
+ struct nlmsghdr *answer = NULL;
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
@@ -1893,7 +1895,7 @@ restore:
ll_init_map(&rth);
- ret = rtnl_talk(&rth, n, n, sizeof(*n));
+ ret = rtnl_talk(&rth, n, &answer);
if ((ret < 0) && (errno == EEXIST))
ret = 0;
diff --git a/ip/iprule.c b/ip/iprule.c
index 8313138..872cfd2 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -393,7 +393,7 @@ static int flush_rule(const struct sockaddr_nl *who, struct nlmsghdr *n,
if (rtnl_open(&rth2, 0) < 0)
return -1;
- if (rtnl_talk(&rth2, n, NULL, 0) < 0)
+ if (rtnl_talk(&rth2, n, NULL) < 0)
return -2;
rtnl_close(&rth2);
@@ -548,12 +548,13 @@ static int restore_handler(const struct sockaddr_nl *nl,
struct nlmsghdr *n, void *arg)
{
int ret;
+ struct nlmsghdr *answer = NULL;
n->nlmsg_flags |= NLM_F_REQUEST | NLM_F_CREATE | NLM_F_ACK;
ll_init_map(&rth);
- ret = rtnl_talk(&rth, n, n, sizeof(*n));
+ ret = rtnl_talk(&rth, n, &answer);
if ((ret < 0) && (errno == EEXIST))
ret = 0;
@@ -760,7 +761,7 @@ static int iprule_modify(int cmd, int argc, char **argv)
if (!table_ok && cmd == RTM_NEWRULE)
req.r.rtm_table = RT_TABLE_MAIN;
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -2;
return 0;
diff --git a/ip/ipseg6.c b/ip/ipseg6.c
index a8f5c69..6d7b1d0 100644
--- a/ip/ipseg6.c
+++ b/ip/ipseg6.c
@@ -125,6 +125,7 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
static int seg6_do_cmd(void)
{
SEG6_REQUEST(req, 1024, opts.cmd, NLM_F_REQUEST);
+ struct nlmsghdr *answer = NULL;
int repl = 0, dump = 0;
if (genl_family < 0) {
@@ -163,12 +164,12 @@ static int seg6_do_cmd(void)
}
if (!repl && !dump) {
- if (rtnl_talk(&grth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&grth, &req.n, NULL) < 0)
return -1;
} else if (repl) {
- if (rtnl_talk(&grth, &req.n, &req.n, sizeof(req)) < 0)
+ if (rtnl_talk(&grth, &req.n, &answer) < 0)
return -2;
- if (process_msg(NULL, &req.n, stdout) < 0) {
+ if (process_msg(NULL, answer, stdout) < 0) {
fprintf(stderr, "Error parsing reply\n");
exit(1);
}
diff --git a/ip/iptoken.c b/ip/iptoken.c
index 1869f76..0528bad 100644
--- a/ip/iptoken.c
+++ b/ip/iptoken.c
@@ -166,7 +166,7 @@ static int iptoken_set(int argc, char **argv, bool delete)
addattr_nest_end(&req.n, afs6);
addattr_nest_end(&req.n, afs);
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return -2;
return 0;
diff --git a/ip/link_gre.c b/ip/link_gre.c
index c2ec5f2..21131ab 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -75,6 +75,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
.i.ifi_family = preferred_family,
.i.ifi_index = ifi->ifi_index,
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *greinfo[IFLA_GRE_MAX + 1];
@@ -98,19 +99,19 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
__u32 fwmark = 0;
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
fprintf(stderr,
"Failed to get existing tunnel info.\n");
return -1;
}
- len = req.n.nlmsg_len;
+ len = answer->nlmsg_len;
len -= NLMSG_LENGTH(sizeof(*ifi));
if (len < 0)
goto get_failed;
- parse_rtattr(tb, IFLA_MAX, IFLA_RTA(&req.i), len);
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(answer + sizeof(struct nlmsghdr)), len);
if (!tb[IFLA_LINKINFO])
goto get_failed;
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 78b5215..98e689e 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -86,6 +86,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
.i.ifi_family = preferred_family,
.i.ifi_index = ifi->ifi_index,
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *greinfo[IFLA_GRE_MAX + 1];
@@ -108,19 +109,19 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
__u32 fwmark = 0;
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
fprintf(stderr,
"Failed to get existing tunnel info.\n");
return -1;
}
- len = req.n.nlmsg_len;
+ len = answer->nlmsg_len;
len -= NLMSG_LENGTH(sizeof(*ifi));
if (len < 0)
goto get_failed;
- parse_rtattr(tb, IFLA_MAX, IFLA_RTA(&req.i), len);
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(NLMSG_DATA(answer)), len);
if (!tb[IFLA_LINKINFO])
goto get_failed;
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 505fb47..19e0ca6 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -83,6 +83,7 @@ static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
.i.ifi_family = preferred_family,
.i.ifi_index = ifi->ifi_index,
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
@@ -103,19 +104,19 @@ static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
__u32 fwmark = 0;
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
fprintf(stderr,
"Failed to get existing tunnel info.\n");
return -1;
}
- len = req.n.nlmsg_len;
+ len = answer->nlmsg_len;
len -= NLMSG_LENGTH(sizeof(*ifi));
if (len < 0)
goto get_failed;
- parse_rtattr(tb, IFLA_MAX, IFLA_RTA(&req.i), len);
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(NLMSG_DATA(answer)), len);
if (!tb[IFLA_LINKINFO])
goto get_failed;
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index d24e737..0ea957b 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -84,6 +84,7 @@ static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
.i.ifi_family = preferred_family,
.i.ifi_index = ifi->ifi_index,
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
@@ -108,19 +109,19 @@ static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
__u32 fwmark = 0;
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
fprintf(stderr,
"Failed to get existing tunnel info.\n");
return -1;
}
- len = req.n.nlmsg_len;
+ len = answer->nlmsg_len;
len -= NLMSG_LENGTH(sizeof(*ifi));
if (len < 0)
goto get_failed;
- parse_rtattr(tb, IFLA_MAX, IFLA_RTA(&req.i), len);
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(NLMSG_DATA(answer)), len);
if (!tb[IFLA_LINKINFO])
goto get_failed;
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 3ffecfa..de15b4a 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -61,6 +61,7 @@ static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
.i.ifi_family = preferred_family,
.i.ifi_index = ifi->ifi_index,
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
@@ -73,19 +74,19 @@ static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
int len;
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
fprintf(stderr,
"Failed to get existing tunnel info.\n");
return -1;
}
- len = req.n.nlmsg_len;
+ len = answer->nlmsg_len;
len -= NLMSG_LENGTH(sizeof(*ifi));
if (len < 0)
goto get_failed;
- parse_rtattr(tb, IFLA_MAX, IFLA_RTA(&req.i), len);
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(NLMSG_DATA(answer)), len);
if (!tb[IFLA_LINKINFO])
goto get_failed;
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 6ea1fc2..663ee03 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -56,6 +56,7 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
.i.ifi_family = preferred_family,
.i.ifi_index = ifi->ifi_index,
};
+ struct nlmsghdr *answer = NULL;
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
@@ -68,19 +69,19 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
int len;
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0) {
+ if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
fprintf(stderr,
"Failed to get existing tunnel info.\n");
return -1;
}
- len = req.n.nlmsg_len;
+ len = answer->nlmsg_len;
len -= NLMSG_LENGTH(sizeof(*ifi));
if (len < 0)
goto get_failed;
- parse_rtattr(tb, IFLA_MAX, IFLA_RTA(&req.i), len);
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(NLMSG_DATA(answer)), len);
if (!tb[IFLA_LINKINFO])
goto get_failed;
diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c
index 8972acd..547d791 100644
--- a/ip/tcp_metrics.c
+++ b/ip/tcp_metrics.c
@@ -306,6 +306,7 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
static int tcpm_do_cmd(int cmd, int argc, char **argv)
{
TCPM_REQUEST(req, 1024, TCP_METRICS_CMD_GET, NLM_F_REQUEST);
+ struct nlmsghdr *answer = NULL;
int atype = -1, stype = -1;
int ack;
@@ -457,12 +458,12 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
}
if (ack) {
- if (rtnl_talk(&grth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&grth, &req.n, NULL) < 0)
return -2;
} else if (atype >= 0) {
- if (rtnl_talk(&grth, &req.n, &req.n, sizeof(req)) < 0)
+ if (rtnl_talk(&grth, &req.n, &answer) < 0)
return -2;
- if (process_msg(NULL, &req.n, stdout) < 0) {
+ if (process_msg(NULL, answer, stdout) < 0) {
fprintf(stderr, "Dump terminated\n");
exit(1);
}
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index de689c4..c794779 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -386,7 +386,7 @@ static int xfrm_policy_modify(int cmd, unsigned int flags, int argc, char **argv
if (req.xpinfo.sel.family == AF_UNSPEC)
req.xpinfo.sel.family = AF_INET;
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
rtnl_close(&rth);
@@ -548,7 +548,7 @@ int xfrm_policy_print(const struct sockaddr_nl *who, struct nlmsghdr *n,
}
static int xfrm_policy_get_or_delete(int argc, char **argv, int delete,
- void *res_nlbuf, size_t res_size)
+ struct nlmsghdr **answer)
{
struct rtnl_handle rth;
struct {
@@ -659,7 +659,7 @@ static int xfrm_policy_get_or_delete(int argc, char **argv, int delete,
(void *)&ctx, ctx.sctx.len);
}
- if (rtnl_talk(&rth, &req.n, res_nlbuf, res_size) < 0)
+ if (rtnl_talk(&rth, &req.n, answer) < 0)
exit(2);
rtnl_close(&rth);
@@ -669,15 +669,14 @@ static int xfrm_policy_get_or_delete(int argc, char **argv, int delete,
static int xfrm_policy_delete(int argc, char **argv)
{
- return xfrm_policy_get_or_delete(argc, argv, 1, NULL, 0);
+ return xfrm_policy_get_or_delete(argc, argv, 1, NULL);
}
static int xfrm_policy_get(int argc, char **argv)
{
- char buf[NLMSG_BUF_SIZE] = {};
- struct nlmsghdr *n = (struct nlmsghdr *)buf;
+ struct nlmsghdr *n = NULL;
- xfrm_policy_get_or_delete(argc, argv, 0, n, sizeof(buf));
+ xfrm_policy_get_or_delete(argc, argv, 0, &n);
if (xfrm_policy_print(NULL, n, (void *)stdout) < 0) {
fprintf(stderr, "An error :-)\n");
@@ -1049,7 +1048,7 @@ static int xfrm_spd_setinfo(int argc, char **argv)
if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
exit(1);
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
rtnl_close(&rth);
@@ -1070,14 +1069,15 @@ static int xfrm_spd_getinfo(int argc, char **argv)
.n.nlmsg_type = XFRM_MSG_GETSPDINFO,
.flags = 0XFFFFFFFF,
};
+ struct nlmsghdr *answer = NULL;
if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
exit(1);
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
exit(2);
- print_spdinfo(&req.n, (void *)stdout);
+ print_spdinfo(answer, (void *)stdout);
rtnl_close(&rth);
@@ -1123,7 +1123,7 @@ static int xfrm_policy_flush(int argc, char **argv)
if (show_stats > 1)
fprintf(stderr, "Flush policy\n");
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
rtnl_close(&rth);
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index 4483fb8..02d3efa 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -726,7 +726,7 @@ static int xfrm_state_modify(int cmd, unsigned int flags, int argc, char **argv)
if (req.xsinfo.family == AF_UNSPEC)
req.xsinfo.family = AF_INET;
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
rtnl_close(&rth);
@@ -757,8 +757,7 @@ static int xfrm_state_allocspi(int argc, char **argv)
char *minp = NULL;
char *maxp = NULL;
struct xfrm_mark mark = {0, 0};
- char res_buf[NLMSG_BUF_SIZE] = {};
- struct nlmsghdr *res_n = (struct nlmsghdr *)res_buf;
+ struct nlmsghdr *answer = NULL;
while (argc > 0) {
if (strcmp(*argv, "mode") == 0) {
@@ -858,10 +857,10 @@ static int xfrm_state_allocspi(int argc, char **argv)
req.xspi.info.family = AF_INET;
- if (rtnl_talk(&rth, &req.n, res_n, sizeof(res_buf)) < 0)
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
exit(2);
- if (xfrm_state_print(NULL, res_n, (void *)stdout) < 0) {
+ if (xfrm_state_print(NULL, answer, (void *)stdout) < 0) {
fprintf(stderr, "An error :-)\n");
exit(1);
}
@@ -1046,16 +1045,15 @@ static int xfrm_state_get_or_delete(int argc, char **argv, int delete)
req.xsid.family = AF_INET;
if (delete) {
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
} else {
- char buf[NLMSG_BUF_SIZE] = {};
- struct nlmsghdr *res_n = (struct nlmsghdr *)buf;
+ struct nlmsghdr *answer = NULL;
- if (rtnl_talk(&rth, &req.n, res_n, sizeof(req)) < 0)
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
exit(2);
- if (xfrm_state_print(NULL, res_n, (void *)stdout) < 0) {
+ if (xfrm_state_print(NULL, answer, (void *)stdout) < 0) {
fprintf(stderr, "An error :-)\n");
exit(1);
}
@@ -1321,14 +1319,15 @@ static int xfrm_sad_getinfo(int argc, char **argv)
.n.nlmsg_type = XFRM_MSG_GETSADINFO,
.flags = 0XFFFFFFFF,
};
+ struct nlmsghdr *answer = NULL;
if (rtnl_open_byproto(&rth, 0, NETLINK_XFRM) < 0)
exit(1);
- if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
exit(2);
- print_sadinfo(&req.n, (void *)stdout);
+ print_sadinfo(answer, (void *)stdout);
rtnl_close(&rth);
@@ -1376,7 +1375,7 @@ static int xfrm_state_flush(int argc, char **argv)
fprintf(stderr, "Flush state with XFRM-PROTO value \"%s\"\n",
strxf_xfrmproto(req.xsf.proto));
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
exit(2);
rtnl_close(&rth);
diff --git a/lib/libgenl.c b/lib/libgenl.c
index 50d2d92..9994469 100644
--- a/lib/libgenl.c
+++ b/lib/libgenl.c
@@ -49,16 +49,17 @@ int genl_resolve_family(struct rtnl_handle *grth, const char *family)
{
GENL_REQUEST(req, 1024, GENL_ID_CTRL, 0, 0, CTRL_CMD_GETFAMILY,
NLM_F_REQUEST);
+ struct nlmsghdr *answer = NULL;
addattr_l(&req.n, sizeof(req), CTRL_ATTR_FAMILY_NAME,
family, strlen(family) + 1);
- if (rtnl_talk(grth, &req.n, &req.n, sizeof(req)) < 0) {
+ if (rtnl_talk(grth, &req.n, &answer) < 0) {
fprintf(stderr, "Error talking to the kernel\n");
return -2;
}
- return genl_parse_getfamily(&req.n);
+ return genl_parse_getfamily(answer);
}
int genl_init_handle(struct rtnl_handle *grth, const char *family,
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 37cfb5a..ff986d4 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -568,7 +568,7 @@ static void rtnl_talk_error(struct nlmsghdr *h, struct nlmsgerr *err,
}
static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
- struct nlmsghdr *answer, size_t maxlen,
+ struct nlmsghdr **answer,
bool show_rtnl_err, nl_ext_ack_fn_t errfn)
{
int status;
@@ -643,8 +643,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
fprintf(stderr, "ERROR truncated\n");
} else if (!err->error) {
if (answer)
- memcpy(answer, h,
- MIN(maxlen, h->nlmsg_len));
+ *answer= h;
return 0;
}
@@ -657,8 +656,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
}
if (answer) {
- memcpy(answer, h,
- MIN(maxlen, h->nlmsg_len));
+ *answer= h;
return 0;
}
@@ -681,22 +679,22 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
}
int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
- struct nlmsghdr *answer, size_t maxlen)
+ struct nlmsghdr **answer)
{
- return __rtnl_talk(rtnl, n, answer, maxlen, true, NULL);
+ return __rtnl_talk(rtnl, n, answer, true, NULL);
}
int rtnl_talk_extack(struct rtnl_handle *rtnl, struct nlmsghdr *n,
- struct nlmsghdr *answer, size_t maxlen,
+ struct nlmsghdr **answer,
nl_ext_ack_fn_t errfn)
{
- return __rtnl_talk(rtnl, n, answer, maxlen, true, errfn);
+ return __rtnl_talk(rtnl, n, answer, true, errfn);
}
int rtnl_talk_suppress_rtnl_errmsg(struct rtnl_handle *rtnl, struct nlmsghdr *n,
- struct nlmsghdr *answer, size_t maxlen)
+ struct nlmsghdr **answer)
{
- return __rtnl_talk(rtnl, n, answer, maxlen, false, NULL);
+ return __rtnl_talk(rtnl, n, answer, false, NULL);
}
int rtnl_listen_all_nsid(struct rtnl_handle *rth)
diff --git a/misc/ss.c b/misc/ss.c
index dd8dfaa..bf21d80 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2597,7 +2597,7 @@ static int kill_inet_sock(struct nlmsghdr *h, void *arg, struct sockstat *s)
raw->sdiag_raw_protocol = s->raw_prot;
}
- return rtnl_talk(rth, &req.nlh, NULL, 0);
+ return rtnl_talk(rth, &req.nlh, NULL);
}
static int show_one_inet_sock(const struct sockaddr_nl *addr,
diff --git a/tc/m_action.c b/tc/m_action.c
index 6ebe85e..12e033b 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -507,14 +507,13 @@ static int tc_action_gd(int cmd, unsigned int flags, int *argc_p, char ***argv_p
req.n.nlmsg_seq = rth.dump = ++rth.seq;
if (cmd == RTM_GETACTION)
- ans = &req.n;
- if (rtnl_talk(&rth, &req.n, ans, MAX_MSG) < 0) {
+ if (rtnl_talk(&rth, &req.n, &ans) < 0) {
fprintf(stderr, "We have an error talking to the kernel\n");
return 1;
}
- if (ans && print_action(NULL, &req.n, (void *)stdout) < 0) {
+ if (ans && print_action(NULL, ans, (void *)stdout) < 0) {
fprintf(stderr, "Dump terminated\n");
return 1;
}
@@ -550,7 +549,7 @@ static int tc_action_modify(int cmd, unsigned int flags, int *argc_p, char ***ar
}
tail->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail;
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0) {
+ if (rtnl_talk(&rth, &req.n, NULL) < 0) {
fprintf(stderr, "We have an error talking to the kernel\n");
ret = -1;
}
@@ -617,7 +616,7 @@ static int tc_act_list_or_flush(int argc, char **argv, int event)
req.n.nlmsg_type = RTM_DELACTION;
req.n.nlmsg_flags |= NLM_F_ROOT;
req.n.nlmsg_flags |= NLM_F_REQUEST;
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0) {
+ if (rtnl_talk(&rth, &req.n, NULL) < 0) {
fprintf(stderr, "We have an error flushing\n");
return 1;
}
diff --git a/tc/tc_class.c b/tc/tc_class.c
index 1a1f1fa..0214775 100644
--- a/tc/tc_class.c
+++ b/tc/tc_class.c
@@ -149,7 +149,7 @@ static int tc_class_modify(int cmd, unsigned int flags, int argc, char **argv)
}
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return 2;
return 0;
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index cf290ae..c43d1b2 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -194,7 +194,7 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv)
}
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0) {
+ if (rtnl_talk(&rth, &req.n, NULL) < 0) {
fprintf(stderr, "We have an error talking to the kernel\n");
return 2;
}
@@ -331,6 +331,7 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
.t.tcm_parent = TC_H_UNSPEC,
.t.tcm_family = AF_UNSPEC,
};
+ struct nlmsghdr *answer = NULL;
struct filter_util *q = NULL;
__u32 prio = 0;
__u32 protocol = 0;
@@ -484,12 +485,12 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
return -1;
}
- if (rtnl_talk(&rth, &req.n, &req.n, MAX_MSG) < 0) {
+ if (rtnl_talk(&rth, &req.n, &answer) < 0) {
fprintf(stderr, "We have an error talking to the kernel\n");
return 2;
}
- print_filter(NULL, &req.n, (void *)stdout);
+ print_filter(NULL, answer, (void *)stdout);
return 0;
}
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index 1e9d909..c52114a 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -190,7 +190,7 @@ static int tc_qdisc_modify(int cmd, unsigned int flags, int argc, char **argv)
req.t.tcm_ifindex = idx;
}
- if (rtnl_talk(&rth, &req.n, NULL, 0) < 0)
+ if (rtnl_talk(&rth, &req.n, NULL) < 0)
return 2;
return 0;
--
2.5.5
^ permalink raw reply related
* [PATCH net] net: qualcomm: rmnet: Fix a double free
From: Dan Carpenter @ 2017-09-08 10:23 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan; +Cc: David S. Miller, netdev, kernel-janitors
This is called from rmnet_map_ingress_handler(). When the
rmnet_map_deaggregate() returns NULL then the caller calls
consume_skb(skb) which frees the skb. The kfree_skb() on this error
path leads to a double free.
Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is from static analysis and not tested.
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 557c9bf1a469..0335fce54201 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -95,10 +95,8 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
skb_pull(skb, packet_len);
/* Some hardware can send us empty frames. Catch them */
- if (ntohs(maph->pkt_len) == 0) {
- kfree_skb(skb);
+ if (ntohs(maph->pkt_len) == 0)
return NULL;
- }
return skbn;
}
^ permalink raw reply related
* [PATCH v2] rtl8xxxu: Don't printk raw binary if serial number is not burned in.
From: Adam Borowski @ 2017-09-08 10:30 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Stefano Brivio
Cc: Adam Borowski
In-Reply-To: <20170908032706.1fa59da9@elisabeth>
I assume that a blank efuse comes with all ones, thus I did not bother
recognizing other possible junk values. This matches 100% of dongles
I've seen (a single Gembird 8192eu).
Signed-off-by: Adam Borowski <kilobyte-b9QjgO8OEXPVItvQsEIGlw@public.gmane.org>
---
v2: strncmp("goofy string") -> memchr_inv()
Stefano Brivio wrote:
> You might want to use memchr_inv():
>
> if (memchr_inv(efuse->serial, 0xff, 11))
> dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
> ...
>
> Mostly cosmetic though.
This looks much better, thanks!
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index 80fee699f58a..38b2ba1ac6f8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -614,7 +614,10 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
- dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
+ if (memchr_inv(efuse->serial, 0xff, 11))
+ dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
+ else
+ dev_info(&priv->udev->dev, "Serial not available.\n");
if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
unsigned char *raw = priv->efuse_wifi.raw;
--
2.14.1
^ permalink raw reply related
* [PATCH net] phy: mvebu-cp110: checking for NULL instead of IS_ERR()
From: Dan Carpenter @ 2017-09-08 10:31 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Antoine Tenart
Cc: netdev, kernel-janitors, linux-kernel, David S. Miller
devm_ioremap_resource() never returns NULL, it only returns error
pointers so this test needs to be changed.
Fixes: d0438bd6aa09 ("phy: add the mvebu cp110 comphy driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This driver apparently is going through the net tree, but netdev isn't
listed as handling it in MAINTAINERS. Kishon, do you know what's up
with that?
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 73ebad6634a7..24578bd68ddc 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -576,8 +576,8 @@ static int mvebu_comphy_probe(struct platform_device *pdev)
return PTR_ERR(priv->regmap);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
priv->base = devm_ioremap_resource(&pdev->dev, res);
- if (!priv->base)
- return -ENOMEM;
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
for_each_available_child_of_node(pdev->dev.of_node, child) {
struct mvebu_comphy_lane *lane;
^ permalink raw reply related
* Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Daniel Borkmann @ 2017-09-08 10:34 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: davem, ast, john.fastabend, andy, netdev
In-Reply-To: <20170908070610.4b8e1df1@redhat.com>
On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:
> On Fri, 8 Sep 2017 00:14:51 +0200
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>
>> + /* This is really only caused by a deliberately crappy
>> + * BPF program, normally we would never hit that case,
>> + * so no need to inform someone via tracepoints either,
>> + * just bail out.
>> + */
>> + if (unlikely(map_owner != xdp_prog))
>> + return -EINVAL;
>
> IMHO we do need to call the tracepoint here. It is not just crappy
> BPF-progs that cause this situation, it is also drivers not implementing
> XDP_REDIRECT yet (which is all but ixgbe). Due to the level XDP
> operates at, tracepoints are the only way users can runtime troubleshoot
> their XDP programs.
Drivers not implementing XDP_REDIRECT don't even get there in
the first place. What they will do is to hit the 'default' case
when they check for the action code from the BPF program. Then
call into bpf_warn_invalid_xdp_action(act), and fall-through
to hit the tracepoint at trace_xdp_exception() which is also
triggered by XDP_ABORTED usually. So when that happens we do
complain loudly and call a tracepoint already. We should probably
tweak the bpf_warn_invalid_xdp_action() message a little to make
it clear that the action could also just be unsupported by the
driver instead of being illegal.
^ permalink raw reply
* Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for generic XDP
From: Daniel Borkmann @ 2017-09-08 10:41 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, Daniel Borkmann, John Fastabend,
Andy Gospodarek, alexei.starovoitov
In-Reply-To: <20170908103601.21cdecb2@redhat.com>
On 09/08/2017 10:36 AM, Jesper Dangaard Brouer wrote:
> On Thu, 07 Sep 2017 16:09:56 +0200
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:
>>> Using bpf_redirect_map is allowed for generic XDP programs, but the
>>> appropriate map lookup was never performed in xdp_do_generic_redirect().
>>>
>>> Instead the map-index is directly used as the ifindex. For the
>>> xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
>>> sending on ifindex 0 which isn't valid, resulting in getting SKB
>>> packets dropped. Thus, the reported performance numbers are wrong in
>>> commit 24251c264798 ("samples/bpf: add option for native and skb mode
>>> for redirect apps") for the 'xdp_redirect_map -S' case.
>>>
>>> It might seem innocent this was lacking, but it can actually crash the
>>> kernel. The potential crash is caused by not consuming redirect_info->map.
>>> The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map
>>> pointer, which will survive even after unloading the xdp bpf_prog and
>>> deallocating the devmap data-structure. This leaves a dead map
>>> pointer around. The kernel will crash when loading the xdp_redirect
>>> sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
>>> and returns XDP_REDIRECT, which will cause it to dereference the map
>>> pointer.
>>>
>>> Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
>>> Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect apps")
>>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>>> ---
>>> include/trace/events/xdp.h | 4 ++--
>>> net/core/filter.c | 14 +++++++++++---
>>> 2 files changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
>>> index 862575ac8da9..4e16c43fba10 100644
>>> --- a/include/trace/events/xdp.h
>>> +++ b/include/trace/events/xdp.h
>>> @@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err,
>>>
>>> #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
>>> trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0, \
>>> - 0, map, idx);
>>> + 0, map, idx)
>>>
>>> #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \
>>> trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0, \
>>> - err, map, idx);
>>> + err, map, idx)
>>>
>>> #endif /* _TRACE_XDP_H */
>>>
>>> diff --git a/net/core/filter.c b/net/core/filter.c
>>> index 5912c738a7b2..3767470cab6c 100644
>>> --- a/net/core/filter.c
>>> +++ b/net/core/filter.c
>>> @@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
>>> struct bpf_prog *xdp_prog)
>>> {
>>> struct redirect_info *ri = this_cpu_ptr(&redirect_info);
>>> + struct bpf_map *map = ri->map;
>>> u32 index = ri->ifindex;
>>> struct net_device *fwd;
>>> unsigned int len;
>>> int err = 0;
>>>
>>> - fwd = dev_get_by_index_rcu(dev_net(dev), index);
>>> ri->ifindex = 0;
>>> + ri->map = NULL;
>>> +
>>> + if (map)
>>> + fwd = __dev_map_lookup_elem(map, index);
>>> + else
>>> + fwd = dev_get_by_index_rcu(dev_net(dev), index);
>>> if (unlikely(!fwd)) {
>>> err = -EINVAL;
>>> goto err;
>>> @@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
>>> }
>>>
>>> skb->dev = fwd;
>>
>> Looks much better above, thanks!
>>
>>> - _trace_xdp_redirect(dev, xdp_prog, index);
>>> + map ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)
>>> + : _trace_xdp_redirect(dev, xdp_prog, index);
>>
>> Could we rather make this in a way such that when the two
>> tracepoints are disabled and thus patched out, that we can
>> also omit the extra conditional which has no purpose then?
>
> First of all I don't think it make much of a difference, I measured the
> impact of the full patch to "cost" 1.62 nanosec (which is arguably
> below the accuracy level of the system under test)
>
> Secondly, I plan to optimize the map case for generic XDP later, where
> I would naturally split this into two functions (as V1, and as
> native-XDP), thus this extra conditional would go away. As I've shown
> offlist (to you, John and Andy) I demonstrated a 24% speedup via a
> xmit_more hack for generic XDP.
Okay, that would be nice indeed to have xmit_more support for
generic XDP as well. If this is going to be split off anyway
later on as in xdp_do_redirect() case, then:
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Phil Sutter @ 2017-09-08 11:02 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, Michal Kubecek
In-Reply-To: <1504865697-27274-2-git-send-email-liuhangbin@gmail.com>
Hi Hangbin,
On Fri, Sep 08, 2017 at 06:14:56PM +0800, Hangbin Liu wrote:
[...]
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index be7ac86..37cfb5a 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle *rth,
> }
> }
>
> +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
> +{
> + struct iovec *iov;
> + int len = -1, buf_len = 32768;
> + char *buffer = *buf;
Isn't it possible to make 'buffer' static instead of the two 'buf'
variables in rtnl_dump_filter_l() and __rtnl_talk()? Then we would have
only a single buffer which is shared between both functions instead of
two which are independently allocated.
> +
> + int flag = MSG_PEEK | MSG_TRUNC;
> +
> + if (buffer == NULL)
> +re_malloc:
> + buffer = malloc(buf_len);
I think using realloc() here is more appropriate since there is no need
to free the buffer in beforehand and calling realloc(NULL, len) is
equivalent to calling malloc(len). I think 'realloc' is also a better
name for the goto label.
> + if (buffer == NULL) {
> + fprintf(stderr, "malloc error: no enough buffer\n");
Minor typo here: s/no/not/
> + return -1;
Return -ENOMEM?
> + }
> +
> + iov = msg->msg_iov;
> + iov->iov_base = buffer;
> + iov->iov_len = buf_len;
> +
> +re_recv:
Just call this 'recv'? (Not really important though.)
> + len = recvmsg(fd, msg, flag);
> +
> + if (len < 0) {
> + if (errno == EINTR || errno == EAGAIN)
> + return 0;
Instead of returning 0 (which makes callers retry), goto re_recv?
> + fprintf(stderr, "netlink receive error %s (%d)\n",
> + strerror(errno), errno);
> + return len;
> + }
> +
> + if (len == 0) {
> + fprintf(stderr, "EOF on netlink\n");
> + return -1;
Return -ENODATA here? (Initially I though about -EOF, but EOF is -1 so
that would be incorrect).
> + }
> +
> + if (len > buf_len) {
> + free(buffer);
If you use realloc() above, this can be dropped.
> + buf_len = len;
For this to work you have to make buf_len static too, otherwise you will
unnecessarily reallocate the buffer. Oh, and that also requires the
single buffer (as pointed out above) because you will otherwise use a
common buf_len for both static buffers passed to this function.
> + flag = 0;
> + goto re_malloc;
> + }
> +
> + if (flag != 0) {
> + flag = 0;
> + goto re_recv;
> + }
> +
> + *buf = buffer;
> + return len;
> +}
> +
> int rtnl_dump_filter_l(struct rtnl_handle *rth,
> const struct rtnl_dump_filter_arg *arg)
> {
> @@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> .msg_iov = &iov,
> .msg_iovlen = 1,
> };
> - char buf[32768];
> + static char *buf = NULL;
If you keep the static buffer in rtnl_recvmsg(), there is no need to
assign NULL here.
> int dump_intr = 0;
>
> - iov.iov_base = buf;
> while (1) {
> int status;
> const struct rtnl_dump_filter_arg *a;
> int found_done = 0;
> int msglen = 0;
>
> - iov.iov_len = sizeof(buf);
> - status = recvmsg(rth->fd, &msg, 0);
> -
> - if (status < 0) {
> - if (errno == EINTR || errno == EAGAIN)
> - continue;
> - fprintf(stderr, "netlink receive error %s (%d)\n",
> - strerror(errno), errno);
> - return -1;
> - }
> -
> - if (status == 0) {
> - fprintf(stderr, "EOF on netlink\n");
> - return -1;
> - }
> + status = rtnl_recvmsg(rth->fd, &msg, &buf);
> + if (status < 0)
> + return status;
> + else if (status == 0)
> + continue;
When retrying inside rtnl_recvmsg(), it won't return 0 anymore. I
believe the whole 'while (1)' loop could go away then.
Everything noted for rtnl_dump_filter_l() applies to __rtnl_talk() as
well.
Thanks, Phil
^ permalink raw reply
* Re: [PATCH iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time
From: Phil Sutter @ 2017-09-08 11:06 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, Michal Kubecek
In-Reply-To: <1504865697-27274-3-git-send-email-liuhangbin@gmail.com>
Hi Hangbin,
On Fri, Sep 08, 2017 at 06:14:57PM +0800, Hangbin Liu wrote:
[...]
> diff --git a/genl/ctrl.c b/genl/ctrl.c
> index 448988e..699657b 100644
> --- a/genl/ctrl.c
> +++ b/genl/ctrl.c
> @@ -55,6 +55,7 @@ int genl_ctrl_resolve_family(const char *family)
> };
> struct nlmsghdr *nlh = &req.n;
> struct genlmsghdr *ghdr = &req.g;
> + struct nlmsghdr *answer = NULL;
I don't think it's necessary to assign NULL here or in any of the other
cases.
> if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC) < 0) {
> fprintf(stderr, "Cannot open generic netlink socket\n");
> @@ -63,19 +64,19 @@ int genl_ctrl_resolve_family(const char *family)
>
> addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, family, strlen(family) + 1);
>
> - if (rtnl_talk(&rth, nlh, nlh, sizeof(req)) < 0) {
> + if (rtnl_talk(&rth, nlh, &answer) < 0) {
I didn't check, but it's likely that at least in some cases 'nlh'
contains some buffer space to hold the answer (if it will be larger than
the input). If so, this could be dropped since the buffer is no longer
reused.
Apart from that, looks good to me!
Thanks, Phil
^ permalink raw reply
* Re: [PATCH net 0/2] netfilter: ipvs: some fixes in sctp_conn_schedule
From: Pablo Neira Ayuso @ 2017-09-08 11:40 UTC (permalink / raw)
To: Simon Horman
Cc: Xin Long, netfilter-devel, Alex Gartrell, lvs-devel, netdev, ja,
wensong
In-Reply-To: <20170831115904.GA16979@verge.net.au>
On Thu, Aug 31, 2017 at 01:59:08PM +0200, Simon Horman wrote:
> On Mon, Aug 28, 2017 at 06:17:32PM +0200, Pablo Neira Ayuso wrote:
> > On Sun, Aug 20, 2017 at 01:38:06PM +0800, Xin Long wrote:
> > > Patch 1/2 fixes the regression introduced by commit 5e26b1b3abce.
> > > Patch 2/2 makes ipvs not create conn for sctp ABORT packet.
> >
> > Will wait for Julian and Simon to tell me what I should do with this.
>
> Hi Pablo,
>
> could you take these directly with Julian's Ack and the following?
>
> Signed-off-by: Simon Horman <horms@verge.net.au>
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH] netfilter: nat: constify rhashtable_params
From: Pablo Neira Ayuso @ 2017-09-08 11:46 UTC (permalink / raw)
To: Arvind Yadav
Cc: davem, fw, kadlec, linux-kernel, netdev, coreteam,
netfilter-devel
In-Reply-To: <d978194ca83e9aa41a330a42442eecd8be9e9a60.1504093452.git.arvind.yadav.cs@gmail.com>
On Wed, Aug 30, 2017 at 05:18:04PM +0530, Arvind Yadav wrote:
> rhashtable_params are not supposed to change at runtime. All
> Functions rhashtable_* working with const rhashtable_params
> provided by <linux/rhashtable.h>. So mark the non-const structs
> as const.
Applied to nf, thanks.
^ permalink raw reply
* Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Jesper Dangaard Brouer @ 2017-09-08 11:52 UTC (permalink / raw)
To: Daniel Borkmann, andy; +Cc: davem, ast, john.fastabend, netdev, brouer
In-Reply-To: <59B27234.4050703@iogearbox.net>
On Fri, 08 Sep 2017 12:34:28 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:
> > On Fri, 8 Sep 2017 00:14:51 +0200
> > Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> >> + /* This is really only caused by a deliberately crappy
> >> + * BPF program, normally we would never hit that case,
> >> + * so no need to inform someone via tracepoints either,
> >> + * just bail out.
> >> + */
> >> + if (unlikely(map_owner != xdp_prog))
> >> + return -EINVAL;
> >
> > IMHO we do need to call the tracepoint here. It is not just crappy
> > BPF-progs that cause this situation, it is also drivers not implementing
> > XDP_REDIRECT yet (which is all but ixgbe). Due to the level XDP
> > operates at, tracepoints are the only way users can runtime troubleshoot
> > their XDP programs.
>
> Drivers not implementing XDP_REDIRECT don't even get there in
> the first place. What they will do is to hit the 'default' case
> when they check for the action code from the BPF program. Then
> call into bpf_warn_invalid_xdp_action(act), and fall-through
> to hit the tracepoint at trace_xdp_exception() which is also
> triggered by XDP_ABORTED usually. So when that happens we do
> complain loudly and call a tracepoint already. We should probably
> tweak the bpf_warn_invalid_xdp_action() message a little to make
> it clear that the action could also just be unsupported by the
> driver instead of being illegal.
Yes. drivers not implementing XDP_REDIRECT will cause a tracepoint
trace_xdp_exception() to be called for its _own_ packets.
But it will still setup and leave map and map_owner pointer dangling.
Another NIC can load an xdp_prog that return XDP_REDIRECT, which will hit
above if-statement, and its packets will disappear, without getting
recorded by a tracepoint (thus hard to debug!).
The fundamental point is that tracepoints is the way we choose to
handle debugging XDP programs. Thus, we must trigger a tracepoint when
a packet gets dropped. Even in this unlikely case.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ 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