* Re: [PATCH v1 RFC 1/7] Replace license with GPL
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-2-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 390 bytes --]
On Fri 2017-10-06 13:32:59, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Replace license with GPL.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
--
(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 v1 RFC 2/7] Clean up code according to patch check suggestions
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-3-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 421 bytes --]
On Fri 2017-10-06 13:33:00, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Clean up code according to patch check suggestions.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(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 v1 RFC 3/7] Rename some functions with ksz9477 prefix
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-4-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 461 bytes --]
On Fri 2017-10-06 13:33:01, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Rename some functions with ksz9477 prefix to separate chip specific code
> from common code.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(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 v1 RFC 4/7] Rename ksz_spi.c to ksz9477_spi.c
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-5-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 550 bytes --]
On Fri 2017-10-06 13:33:02, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to add
> more KSZ switch drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
This will ask people question they already answered, but I guess
that's ok.
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(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 v1 RFC 5/7] Break KSZ9477 DSA driver into two files
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-6-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 634 bytes --]
On Fri 2017-10-06 13:33:03, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Break KSZ9477 DSA driver into two files in preparation to add more KSZ
> switch drivers.
> Add common functions in ksz_common.h so that other KSZ switch drivers
> can access code in ksz_common.c.
> Add ksz_spi.h for common functions used by KSZ switch SPI drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(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 v1 RFC 6/7] Add MIB counter reading support
From: Pavel Machek @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver, netdev,
linux-kernel
In-Reply-To: <1507321985-15097-7-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]
Hi!
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add MIB counter reading support.
> Rename ksz_9477_reg.h to ksz9477_reg.h for consistency as the product
> name is always KSZ####.
> Header file ksz_priv.h no longer contains any chip specific data.
Nothing obviously wrong here, but if you are doing another iteration,
it would be nice to separate "Rename ksz_9477_reg.h to ksz9477_reg.h"
from the code changes.
Best regards,
> + timeout = 1000;
> + do {
> + ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
> + &data);
> + usleep_range(1, 10);
> + if (!(data & MIB_COUNTER_READ))
> + break;
> + } while (timeout-- > 0);
1000 iterations, 1usec each, so 1msec, but you allow up to 10 msec. Interesting.
> + /* failed to read MIB. get out of loop */
> + if (!timeout) {
> + dev_dbg(dev->dev, "Failed to get MIB\n");
> + return;
> + }
Are you sure this works? AFAICT "timeout" will underflow, so !timeout
will not trigger.
> -static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
> - uint64_t *buf)
> -{
> - struct ksz_device *dev = ds->priv;
> - int i;
> - u32 data;
> - int timeout;
> -
> - mutex_lock(&dev->stats_mutex);
> -
> - for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> - data = MIB_COUNTER_READ;
> - data |= ((mib_names[i].index & 0xFF) << MIB_COUNTER_INDEX_S);
> - ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
> -
> - timeout = 1000;
> - do {
> - ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
> - &data);
> - usleep_range(1, 10);
> - if (!(data & MIB_COUNTER_READ))
> - break;
> - } while (timeout-- > 0);
> -
> - /* failed to read MIB. get out of loop */
> - if (!timeout) {
> - dev_dbg(dev->dev, "Failed to get MIB\n");
> - break;
> - }
Hmm. Bug was there already...
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 v1 RFC 2/7] Clean up code according to patch check suggestions
From: Florian Fainelli @ 2017-10-09 19:54 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-3-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Clean up code according to patch check suggestions.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v1 RFC 3/7] Rename some functions with ksz9477 prefix
From: Florian Fainelli @ 2017-10-09 19:56 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-4-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Rename some functions with ksz9477 prefix to separate chip specific code
> from common code.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: linux-next: manual merge of the drivers-x86 tree with the net-next tree
From: Mark Brown @ 2017-10-09 19:56 UTC (permalink / raw)
To: Mika Westerberg
Cc: Darren Hart, Mario Limonciello, Yehezkel Bernat, Andy Shevchenko,
Amir Levy, Michael Jamet, David S. Miller, netdev,
Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20171009194301.GP2761@lahna.fi.intel.com>
[-- Attachment #1: Type: text/plain, Size: 390 bytes --]
On Mon, Oct 09, 2017 at 10:43:01PM +0300, Mika Westerberg wrote:
> If possible, I would rather move this chapter to be before "Networking
> over Thunderbolt cable". Reason is that it then follows NVM flashing
> chapter which is typically where you need to force power in the first
> place.
I guess that's something best sorted out either in the relevant trees or
during the merge window?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v1 RFC 4/7] Rename ksz_spi.c to ksz9477_spi.c
From: Florian Fainelli @ 2017-10-09 19:56 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-5-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to add
> more KSZ switch drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v1 RFC 0/7] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers
From: Florian Fainelli @ 2017-10-09 19:58 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-1-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:32 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> This series of patches is to modify the original KSZ9477 DSA driver so
> that other KSZ switch drivers can be added and use the common code.
>
> There are several steps to accomplish this achievement. First is to
> rename some function names with a prefix to indicate chip specific
> function. Second is to move common code into header that can be shared.
> Last is to modify tag_ksz.c so that it can handle many tail tag formats
> used by different KSZ switch drivers.
>
> ksz_common.c will contain the common code used by all KSZ switch drivers.
> ksz9477.c will contain KSZ9477 code from the original ksz_common.c.
> ksz9477_spi.c is renamed from ksz_spi.c.
> ksz9477_reg.h is renamed from ksz_9477_reg.h.
> ksz_common.h is added to provide common code access to KSZ switch
> drivers.
> ksz_spi.h is added to provide common SPI access functions to KSZ SPI
> drivers.
Most of this looks fine, and it is probably time to move away from the
RFC state and do a formal patch submission with the Reviewed-by and
Acked-by tags you just collected.
Can you also make sure you prefix your patches with: net: dsa: to be
consistent with other submissions done to that subsystem, e.g:
net: dsa: microchip: Replace license with GPL
Thank you!
>
> v1
> - Each patch in the set is self-contained
> - Use ksz9477 prefix to indicate KSZ9477 specific code
> - Simplify MIB counter reading code
> - Switch driver code is not accessed from tag_ksz.c
>
> Tristram Ha (7):
> Replace license with GPL.
> Clean up code according to patch check suggestions.
> Rename some functions with ksz9477 prefix to separate chip specific
> code from common code.
> Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to
> add more KSZ switch drivers.
> Break KSZ9477 DSA driver into two files in preparation to add more KSZ
> switch drivers. Add common functions in ksz_common.h so that other
> KSZ switch drivers can access code in ksz_common.c. Add ksz_spi.h
> for common functions used by KSZ switch SPI drivers.
> Add MIB counter reading support. Rename ksz_9477_reg.h to
> ksz9477_reg.h for consistency as the product name is always KSZ####.
> Header file ksz_priv.h no longer contains any chip specific data.
> Modify tag_ksz.c so that tail tag code can be used by other KSZ switch
> drivers.
>
> drivers/net/dsa/microchip/Kconfig | 14 +-
> drivers/net/dsa/microchip/Makefile | 4 +-
> drivers/net/dsa/microchip/ksz9477.c | 1376 ++++++++++++++++++++
> .../microchip/{ksz_9477_reg.h => ksz9477_reg.h} | 23 +-
> drivers/net/dsa/microchip/ksz9477_spi.c | 188 +++
> drivers/net/dsa/microchip/ksz_common.c | 1210 ++++-------------
> drivers/net/dsa/microchip/ksz_common.h | 234 ++++
> drivers/net/dsa/microchip/ksz_priv.h | 258 ++--
> drivers/net/dsa/microchip/ksz_spi.c | 216 ---
> drivers/net/dsa/microchip/ksz_spi.h | 82 ++
> include/net/dsa.h | 2 +-
> net/dsa/Kconfig | 4 +
> net/dsa/dsa.c | 4 +-
> net/dsa/dsa_priv.h | 2 +-
> net/dsa/tag_ksz.c | 107 +-
> 15 files changed, 2331 insertions(+), 1393 deletions(-)
> create mode 100644 drivers/net/dsa/microchip/ksz9477.c
> rename drivers/net/dsa/microchip/{ksz_9477_reg.h => ksz9477_reg.h} (98%)
> create mode 100644 drivers/net/dsa/microchip/ksz9477_spi.c
> create mode 100644 drivers/net/dsa/microchip/ksz_common.h
> delete mode 100644 drivers/net/dsa/microchip/ksz_spi.c
> create mode 100644 drivers/net/dsa/microchip/ksz_spi.h
>
--
Florian
^ permalink raw reply
* RE: [PATCH v1 RFC 1/7] Replace license with GPL
From: Woojung.Huh @ 2017-10-09 19:58 UTC (permalink / raw)
To: Tristram.Ha, andrew, f.fainelli, pavel, ruediger.schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-2-git-send-email-Tristram.Ha@microchip.com>
> Subject: [PATCH v1 RFC 1/7] Replace license with GPL
>
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Replace license with GPL.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Woojung Huh <Woojung.Huh@microchip.com>
^ permalink raw reply
* Re: [PATCH v1 RFC 5/7] Break KSZ9477 DSA driver into two files
From: Florian Fainelli @ 2017-10-09 20:01 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-6-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Break KSZ9477 DSA driver into two files in preparation to add more KSZ
> switch drivers.
> Add common functions in ksz_common.h so that other KSZ switch drivers
> can access code in ksz_common.c.
> Add ksz_spi.h for common functions used by KSZ switch SPI drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v1 RFC 6/7] Add MIB counter reading support
From: Florian Fainelli @ 2017-10-09 20:07 UTC (permalink / raw)
To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
netdev, linux-kernel
In-Reply-To: <1507321985-15097-7-git-send-email-Tristram.Ha@microchip.com>
On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add MIB counter reading support.
> Rename ksz_9477_reg.h to ksz9477_reg.h for consistency as the product
> name is always KSZ####.
> Header file ksz_priv.h no longer contains any chip specific data.
You should probably explain in the commit message that you are adding a
timer that reads counters every 30 seconds to avoid overflows, as this
is a pretty important piece of information.
> +static void ksz9477_phy_setup(struct ksz_device *dev, int port,
> + struct phy_device *phy)
> +{
> + if (port < dev->phy_port_cnt) {
> + /* SUPPORTED_Asym_Pause and SUPPORTED_Pause can be removed to
> + * disable flow control when rate limiting is used.
> + */
> + phy->advertising = phy->supported;
> + }
> +}
This appears to be an unrelated change here that you would want to put
in a separate patch.
> +
> static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
> {
> u8 data8;
> @@ -1159,6 +1198,8 @@ static int ksz9477_setup(struct dsa_switch *ds)
> /* start switch */
> ksz_cfg(dev, REG_SW_OPERATION, SW_START, true);
>
> + ksz_init_mib_timer(dev);
> +
> return 0;
> }
>
> @@ -1168,6 +1209,7 @@ static int ksz9477_setup(struct dsa_switch *ds)
> .set_addr = ksz9477_set_addr,
> .phy_read = ksz9477_phy_read16,
> .phy_write = ksz9477_phy_write16,
> + .adjust_link = ksz_adjust_link,
> .port_enable = ksz_enable_port,
> .port_disable = ksz_disable_port,
> .get_strings = ksz9477_get_strings,
> @@ -1289,6 +1331,7 @@ static int ksz9477_switch_init(struct ksz_device *dev)
> if (!dev->ports)
> return -ENOMEM;
> for (i = 0; i < dev->mib_port_cnt; i++) {
> + mutex_init(&dev->ports[i].mib.cnt_mutex);
> dev->ports[i].mib.counters =
> devm_kzalloc(dev->dev,
> sizeof(u64) *
> @@ -1310,7 +1353,12 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
> .get_port_addr = ksz9477_get_port_addr,
> .cfg_port_member = ksz9477_cfg_port_member,
> .flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
> + .phy_setup = ksz9477_phy_setup,
> .port_setup = ksz9477_port_setup,
> + .r_mib_cnt = ksz9477_r_mib_cnt,
> + .r_mib_pkt = ksz9477_r_mib_pkt,
> + .freeze_mib = ksz9477_freeze_mib,
> + .port_init_cnt = ksz9477_port_init_cnt,
> .shutdown = ksz9477_reset_switch,
> .detect = ksz9477_switch_detect,
> .init = ksz9477_switch_init,
> diff --git a/drivers/net/dsa/microchip/ksz_9477_reg.h b/drivers/net/dsa/microchip/ksz9477_reg.h
> similarity index 100%
> rename from drivers/net/dsa/microchip/ksz_9477_reg.h
> rename to drivers/net/dsa/microchip/ksz9477_reg.h
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 1c9c4c5..885b3e3 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -50,6 +50,81 @@ void ksz_update_port_member(struct ksz_device *dev, int port)
> }
> }
>
> +static void port_r_cnt(struct ksz_device *dev, int port)
> +{
> + struct ksz_port_mib *mib = &dev->ports[port].mib;
> + u64 *dropped;
> +
> + /* Some ports may not have MIB counters before SWITCH_COUNTER_NUM. */
> + while (mib->cnt_ptr < dev->reg_mib_cnt) {
> + dev->dev_ops->r_mib_cnt(dev, port, mib->cnt_ptr,
> + &mib->counters[mib->cnt_ptr]);
> + ++mib->cnt_ptr;
> + }
> +
> + /* last one in storage */
> + dropped = &mib->counters[dev->mib_cnt];
> +
> + /* Some ports may not have MIB counters after SWITCH_COUNTER_NUM. */
> + while (mib->cnt_ptr < dev->mib_cnt) {
> + dev->dev_ops->r_mib_pkt(dev, port, mib->cnt_ptr,
> + dropped, &mib->counters[mib->cnt_ptr]);
> + ++mib->cnt_ptr;
> + }
> + mib->cnt_ptr = 0;
> +}
> +
> +static void ksz_mib_read_work(struct work_struct *work)
> +{
> + struct ksz_device *dev =
> + container_of(work, struct ksz_device, mib_read);
> + struct ksz_port *p;
> + struct ksz_port_mib *mib;
> + int i;
> +
> + for (i = 0; i < dev->mib_port_cnt; i++) {
> + p = &dev->ports[i];
> + if (!p->on)
> + continue;
> + mib = &p->mib;
> + mutex_lock(&mib->cnt_mutex);
> +
> + /* read only dropped counters when link is not up */
> + if (p->link_down)
> + p->link_down = 0;
> + else if (!p->link_up)
> + mib->cnt_ptr = dev->reg_mib_cnt;
Can't you just check the ports' PHY device here instead of caching that
(which can get out of sync)?
> +void ksz_adjust_link(struct dsa_switch *ds, int port,
> + struct phy_device *phydev)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz_port *p = &dev->ports[port];
> +
> + if (phydev->link) {
> + p->speed = phydev->speed;
> + p->duplex = phydev->duplex;
> + p->flow_ctrl = phydev->pause;
> + p->link_up = 1;
> + dev->live_ports |= (1 << port) & dev->on_ports;
> + } else if (p->link_up) {
> + p->link_up = 0;
> + p->link_down = 1;
> + dev->live_ports &= ~(1 << port);
> + }
> +}
It would have been nice to explain why this is needed in the commit
message. dev->live_ports is read-only it seems?
--
Florian
^ permalink raw reply
* RE: [net-next 14/15] i40e: ignore skb->xmit_more when deciding to set RS bit
From: Keller, Jacob E @ 2017-10-09 20:10 UTC (permalink / raw)
To: Alexander Duyck, David Laight
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <CAKgT0UfZEU++hZkpp1NixYxj=2STfXX-5opz2BoEhG1SfUA5pQ@mail.gmail.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Alexander Duyck
> Sent: Monday, October 09, 2017 9:28 AM
> To: David Laight <David.Laight@aculab.com>
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net;
> Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 14/15] i40e: ignore skb->xmit_more when deciding to set
> RS bit
>
> On Mon, Oct 9, 2017 at 2:07 AM, David Laight <David.Laight@aculab.com> wrote:
> > From: Jeff Kirsher
> >> Sent: 06 October 2017 18:57
> >> From: Jacob Keller <jacob.e.keller@intel.com>
> >>
> >> Since commit 6a7fded776a7 ("i40e: Fix RS bit update in Tx path and
> >> disable force WB workaround") we've tried to "optimize" setting the
> >> RS bit based around skb->xmit_more. This same logic was refactored
> >> in commit 1dc8b538795f ("i40e: Reorder logic for coalescing RS bits"),
> >> but ultimately was not functionally changed.
> >
> > I've tried to understand the above, but without definitions of WB
> > and RS and the '4-ness' of something it is quite difficult.
> >
> > I THINK this is all about telling the hardware there is a packet
> > in the ring to transmit?
> >
> > I don't understand the 4-ness. Linux requires that the hardware
> > be notified of a single packet transmit, and that the 'transmit
> > complete' also be notified in 'a timely manner' for a single packet.
>
> So to clarify some of this. The RS is short for Report Status. It
> tells the hardware that when it has completed the descriptor it should
> trigger a write back, aka WB.
>
> The 4-ness is because each descriptor is 16 bytes, so if we write back
> once every 4 descriptors that is one 64B cache line which is much
> better for most platforms performance wise.
>
> > skb->xmit_more ought to be usable to optimise both of these
> > (assuming it isn't a lie).
>
> Actually it leads to issues if we hold off for too long. If we are
> busy dequeueing a long list, and the Tx queue has gone empty then we
> are stalling Tx without need to do so. We need to be regularly
> notifying the device that it has buffers available to transmit which
> is what xmit_more is good for. However in addition the hardware needs
> to notify us regularly that there are buffers ready to clean which it
> isn't necessarily so useful for, especially if are filling the entire
> ring and only providing one notification to the driver that there are
> buffers ready since we can defer the Tx interrupt for too long.
>
> What Jake is trying to fix here is to prevent us from generating what
> looks like a square wave in terms of throughput. Essentially the
> current behavior that is being seen is that all the buffers in the
> ring either belong to software, or belong to hardware. It is best for
> us to have this split half and half so that the hardware has buffers
> to transmit and software has buffers to clean and enqueue new buffers
> onto.
>
Yes this sums it up pretty well. The test case which found this bug is outlined in the description. Essentially we could see up to dozens or even almost a hundred packets in a row bunched up if we had a bunch of virtual devices layered on top of the network driver. This led to us not indicating to get status from the hardware about finished packets, which results in an increased delay to finish Tx.
> > The driver would need to ensure a ring full of messages isn't
> > generated without either wakeup - but that might be 128 frames.
>
> That is what is happening here. Basically we are guaranteeing
> writebacks are occurring on a regular basis instead of in large
> bursts.
>
> > FWIW on the systems I have PCIe writes are relatively cheap
> > (reads are slow). So different counts would be appropriate
> > for delaying doorbell writes and requesting completion interrupts.
> >
> > David
>
> The doorbell writes are still being delayed the same amount with this
> patch. The only thing that was split out was the device write backs
> are now occurring on a more regular basis instead of being held off
> until a much larger set is handled.
>
Exactly. The tail bumps are still functioning the same, but it's the hardware report status requests which are not delayed. Note that there's some further quirks in how our hardware performs write backs which exacerbate the problem because of how the cachelines are setup so that the delay is even worse than we would expect.
Thanks,
Jake
> - Alex
^ permalink raw reply
* Re: [PATCHv4 iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time
From: Phil Sutter @ 2017-10-09 20:25 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Hangbin Liu, netdev, Michal Kubecek, Hangbin Liu
In-Reply-To: <20171002103708.0572704b@xeon-e3>
Hi Stephen,
On Mon, Oct 02, 2017 at 10:37:08AM -0700, Stephen Hemminger wrote:
> On Thu, 28 Sep 2017 21:33:46 +0800
> Hangbin Liu <haliu@redhat.com> wrote:
>
> > From: Hangbin Liu <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.
> >
> > We need to free answer after using.
> >
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
>
> Most of the uses of rtnl_talk() don't need to this peek and dynamic sizing.
> Can only those places that need that be targeted?
We could probably do that, by having a buffer on stack in __rtnl_talk()
which will be used instead of the allocated one if 'answer' is NULL. Or
maybe even introduce a dedicated API call for the dynamically allocated
receive buffer. But I really doubt that's feasible: AFAICT, that stack
buffer still needs to be reasonably sized since the reply might be
larger than the request (reusing the request buffer would be the most
simple way to tackle this), also there is support for extack which may
bloat the response to arbitrary size. Hangbin has shown in his benchmark
that the overhead of the second syscall is negligible, so why care about
that and increase code complexity even further?
Not saying it's not possible, but I just doubt it's worth the effort.
Cheers, Phil
^ permalink raw reply
* Re: [PATCH v2] net/core: Fix BUG to BUG_ON conditionals.
From: Levin, Alexander (Sasha Levin) @ 2017-10-09 20:26 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Tim Hansen, davem@davemloft.net, willemb@google.com,
edumazet@google.com, soheil@google.com, pabeni@redhat.com,
elena.reshetova@intel.com, tom@quantonium.net, Jason@zx2c4.com,
fw@strlen.de, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20171009171540.lgmk2slq3bdrptxk@ast-mbp.dhcp.thefacebook.com>
On Mon, Oct 09, 2017 at 10:15:42AM -0700, Alexei Starovoitov wrote:
>On Mon, Oct 09, 2017 at 11:37:59AM -0400, Tim Hansen wrote:
>> Fix BUG() calls to use BUG_ON(conditional) macros.
>>
>> This was found using make coccicheck M=net/core on linux next
>> tag next-2017092
>>
>> Signed-off-by: Tim Hansen <devtimhansen@gmail.com>
>> ---
>> net/core/skbuff.c | 15 ++++++---------
>> 1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index d98c2e3ce2bf..34ce4c1a0f3c 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -1350,8 +1350,7 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t gfp_mask)
>> /* Set the tail pointer and length */
>> skb_put(n, skb->len);
>>
>> - if (skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len))
>> - BUG();
>> + BUG_ON(skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len));
>
>I'm concerned with this change.
>1. Calling non-trivial bit of code inside the macro is a poor coding style (imo)
>2. BUG_ON != BUG. Some archs like mips and ppc have HAVE_ARCH_BUG_ON and implementation
>of BUG and BUG_ON look quite different.
For these archs, wouldn't it then be more efficient to use BUG_ON rather than BUG()?
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH] once: switch to new jump label API
From: Eric Biggers @ 2017-10-09 20:27 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David S. Miller, netdev, linux-kernel, Jason Baron,
Peter Zijlstra, Eric Biggers
In-Reply-To: <20170916040751.GA14238@zzz.localdomain>
On Fri, Sep 15, 2017 at 09:07:51PM -0700, Eric Biggers wrote:
> On Tue, Aug 22, 2017 at 02:44:41PM -0400, Hannes Frederic Sowa wrote:
> > Eric Biggers <ebiggers3@gmail.com> writes:
> >
> > > From: Eric Biggers <ebiggers@google.com>
> > >
> > > Switch the DO_ONCE() macro from the deprecated jump label API to the new
> > > one. The new one is more readable, and for DO_ONCE() it also makes the
> > > generated code more icache-friendly: now the one-time initialization
> > > code is placed out-of-line at the jump target, rather than at the inline
> > > fallthrough case.
> > >
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> >
> > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org.
> >
> > Thanks!
>
> Great! Who though is the maintainer for this code? It seems it was originally
> taken by David Miller through the networking tree. David, are you taking
> further patches to the "once" functions, or should I be trying to get this into
> -mm, or somewhere else?
>
> Eric
Ping.
^ permalink raw reply
* [net-next 0/3] ip_gre: a bunch of fixes for mtu
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
To: netdev
The first two patches are to fix some issues for mtu and needed_headroom length
calculation from the gre and erspan tunnel header. The last path tries to avoid
arithmetic operation for every packet when checking for erspan truncate.
William Tu (3):
ip_gre: fix mtu and headroom size
ip_gre: fix erspan tunnel mtu calculation
ip_gre: cache the device mtu hard_header_len calc
include/net/erspan.h | 1 +
include/net/ip_tunnels.h | 1 +
net/ipv4/ip_gre.c | 23 +++++++++++------------
net/ipv4/ip_tunnel.c | 3 +++
4 files changed, 16 insertions(+), 12 deletions(-)
--
2.7.4
^ permalink raw reply
* [net-next 1/3] ip_gre: fix mtu and headroom size
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
To: netdev; +Cc: Tom Herbert
In-Reply-To: <1507582067-36718-1-git-send-email-u9012063@gmail.com>
The ip_gre_calc_hlen() already counts gre's base header len (4-byte)
plus optional header's len, so tunnel->hlen has the entire gre
headeri + options len. Thus, remove the -4 and +4 when calculating
the needed_headroom and mtu.
Fixes: 4565e9919cda ("gre: Setup and TX path for gre/UDP foo-over-udp encapsulation")
Signed-off-by: William Tu <u9012063@gmail.com>
Cc: Tom Herbert <tom@quantonium.net>
---
net/ipv4/ip_gre.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index c105a315b1a3..286065c35959 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -947,8 +947,8 @@ static void __gre_tunnel_init(struct net_device *dev)
t_hlen = tunnel->hlen + sizeof(struct iphdr);
- dev->needed_headroom = LL_MAX_HEADER + t_hlen + 4;
- dev->mtu = ETH_DATA_LEN - t_hlen - 4;
+ dev->needed_headroom = LL_MAX_HEADER + t_hlen;
+ dev->mtu = ETH_DATA_LEN - t_hlen;
dev->features |= GRE_FEATURES;
dev->hw_features |= GRE_FEATURES;
--
2.7.4
^ permalink raw reply related
* [net-next 2/3] ip_gre: fix erspan tunnel mtu calculation
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
To: netdev; +Cc: Xin Long
In-Reply-To: <1507582067-36718-1-git-send-email-u9012063@gmail.com>
Remove the unnecessary -4 and +4 bytes at mtu and headroom calculation.
In addition, erspan uses fixed 8-byte gre header, so add ERSPAN_GREHDR_LEN
macro for better readability.
Now tunnel->hlen = grehdr(8) + erspanhdr(8) = 16 byte.
The mtu should be ETH_DATA_LEN - 16 - iph(20) = 1464.
After the ip_tunnel_bind_dev(), the mtu is adjusted to
1464 - 14 (dev->hard_header_len) = 1450.
The maximum skb->len the erspan tunnel can carry without
being truncated is 1450 + 14 = 1464 byte.
Signed-off-by: William Tu <u9012063@gmail.com>
Cc: Xin Long <lucien.xin@gmail.com>
---
include/net/erspan.h | 1 +
net/ipv4/ip_gre.c | 11 +++++------
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/net/erspan.h b/include/net/erspan.h
index ca94fc86865e..e28294e248d0 100644
--- a/include/net/erspan.h
+++ b/include/net/erspan.h
@@ -28,6 +28,7 @@
*/
#define ERSPAN_VERSION 0x1
+#define ERSPAN_GREHDR_LEN 8 /* ERSPAN has fixed 8-byte GRE header */
#define VER_MASK 0xf000
#define VLAN_MASK 0x0fff
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 286065c35959..6e6e4c4811cc 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -569,8 +569,7 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
key = &tun_info->key;
- /* ERSPAN has fixed 8 byte GRE header */
- tunnel_hlen = 8 + sizeof(struct erspanhdr);
+ tunnel_hlen = ERSPAN_GREHDR_LEN + sizeof(struct erspanhdr);
rt = prepare_fb_xmit(skb, dev, &fl, tunnel_hlen);
if (!rt)
@@ -591,7 +590,7 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
erspan_build_header(skb, tunnel_id_to_key32(key->tun_id),
ntohl(md->index), truncate);
- gre_build_header(skb, 8, TUNNEL_SEQ,
+ gre_build_header(skb, ERSPAN_GREHDR_LEN, TUNNEL_SEQ,
htons(ETH_P_ERSPAN), 0, htonl(tunnel->o_seqno++));
df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
@@ -1242,14 +1241,14 @@ static int erspan_tunnel_init(struct net_device *dev)
struct ip_tunnel *tunnel = netdev_priv(dev);
int t_hlen;
- tunnel->tun_hlen = 8;
+ tunnel->tun_hlen = ERSPAN_GREHDR_LEN;
tunnel->parms.iph.protocol = IPPROTO_GRE;
tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen +
sizeof(struct erspanhdr);
t_hlen = tunnel->hlen + sizeof(struct iphdr);
- dev->needed_headroom = LL_MAX_HEADER + t_hlen + 4;
- dev->mtu = ETH_DATA_LEN - t_hlen - 4;
+ dev->needed_headroom = LL_MAX_HEADER + t_hlen;
+ dev->mtu = ETH_DATA_LEN - t_hlen;
dev->features |= GRE_FEATURES;
dev->hw_features |= GRE_FEATURES;
dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
--
2.7.4
^ permalink raw reply related
* [net-next 3/3] ip_gre: cache the device mtu hard_header_len calc
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
To: netdev; +Cc: David Laight
In-Reply-To: <1507582067-36718-1-git-send-email-u9012063@gmail.com>
The patch introduces ip_tunnel->ether_mtu fields to cache the value of
dev->mtu + dev->hard_header_len. This avoids the arithmetic operation
on every packet.
Signed-off-by: William Tu <u9012063@gmail.com>
Cc: David Laight <David.Laight@aculab.com>
---
include/net/ip_tunnels.h | 1 +
net/ipv4/ip_gre.c | 8 ++++----
net/ipv4/ip_tunnel.c | 3 +++
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index b41a1e057fce..19565be26e13 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -117,6 +117,7 @@ struct ip_tunnel {
/* This field used only by ERSPAN */
u32 index; /* ERSPAN type II index */
+ unsigned int ether_mtu; /* The mtu including the ether hdr */
struct dst_cache dst_cache;
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 6e6e4c4811cc..994b8ddea0b1 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -578,8 +578,8 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
if (gre_handle_offloads(skb, false))
goto err_free_rt;
- if (skb->len > dev->mtu + dev->hard_header_len) {
- pskb_trim(skb, dev->mtu + dev->hard_header_len);
+ if (skb->len > tunnel->ether_mtu) {
+ pskb_trim(skb, tunnel->ether_mtu);
truncate = true;
}
@@ -730,8 +730,8 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
if (skb_cow_head(skb, dev->needed_headroom))
goto free_skb;
- if (skb->len > dev->mtu + dev->hard_header_len) {
- pskb_trim(skb, dev->mtu + dev->hard_header_len);
+ if (skb->len > tunnel->ether_mtu) {
+ pskb_trim(skb, tunnel->ether_mtu);
truncate = true;
}
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index fe6fee728ce4..859af5b86802 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -348,6 +348,7 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
dev->needed_headroom = t_hlen + hlen;
mtu -= (dev->hard_header_len + t_hlen);
+ tunnel->ether_mtu = mtu + dev->hard_header_len;
if (mtu < 68)
mtu = 68;
@@ -952,6 +953,7 @@ int __ip_tunnel_change_mtu(struct net_device *dev, int new_mtu, bool strict)
}
dev->mtu = new_mtu;
+ tunnel->ether_mtu = new_mtu + dev->hard_header_len;
return 0;
}
EXPORT_SYMBOL_GPL(__ip_tunnel_change_mtu);
@@ -1183,6 +1185,7 @@ int ip_tunnel_init(struct net_device *dev)
tunnel->dev = dev;
tunnel->net = dev_net(dev);
+ tunnel->ether_mtu = dev->mtu + dev->hard_header_len;
strcpy(tunnel->parms.name, dev->name);
iph->version = 4;
iph->ihl = 5;
--
2.7.4
^ permalink raw reply related
* [PATCH 0/3] net-ATM: Adjustments for several function implementations
From: SF Markus Elfring @ 2017-10-09 20:48 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, Andrew Morton, Augusto Mecking Caringi,
Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
Hans Liljestrand, Jarod Wilson, Johannes Berg, Kees Cook,
Mitchell Blank Jr, Roopa Prabhu
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 9 Oct 2017 22:30:33 +0200
Some update suggestions were taken into account
from static source code analysis.
Markus Elfring (3):
Delete an error message for a failed memory allocation in five functions
Improve a size determination in 12 functions
Adjust 121 checks for null pointers
net/atm/addr.c | 2 +-
net/atm/br2684.c | 19 ++++----
net/atm/clip.c | 6 +--
net/atm/lec.c | 49 ++++++++++----------
net/atm/mpc.c | 126 ++++++++++++++++++++++++--------------------------
net/atm/mpoa_caches.c | 75 +++++++++++++++---------------
net/atm/pppoatm.c | 11 ++---
net/atm/proc.c | 2 +-
net/atm/signaling.c | 2 +-
9 files changed, 141 insertions(+), 151 deletions(-)
--
2.14.2
^ permalink raw reply
* [PATCH 1/3] net/atm: Delete an error message for a failed memory allocation in five functions
From: SF Markus Elfring @ 2017-10-09 20:49 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, Andrew Morton, Augusto Mecking Caringi,
Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
Hans Liljestrand, Jarod Wilson, Johannes Berg, Kees Cook,
Mitchell Blank Jr, Roopa Prabhu
Cc: LKML, kernel-janitors
In-Reply-To: <10a92558-1105-b947-86a2-6ac763cca36d@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 9 Oct 2017 21:34:35 +0200
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/atm/lec.c | 5 ++---
net/atm/mpc.c | 8 ++------
net/atm/mpoa_caches.c | 8 ++------
3 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/net/atm/lec.c b/net/atm/lec.c
index a3d93a1bb133..f5be0b931978 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -1553,10 +1553,9 @@ static struct lec_arp_table *make_entry(struct lec_priv *priv,
struct lec_arp_table *to_return;
to_return = kzalloc(sizeof(struct lec_arp_table), GFP_ATOMIC);
- if (!to_return) {
- pr_info("LEC: Arp entry kmalloc failed\n");
+ if (!to_return)
return NULL;
- }
+
ether_addr_copy(to_return->mac_addr, mac_addr);
INIT_HLIST_NODE(&to_return->next);
setup_timer(&to_return->timer, lec_arp_expire_arp,
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index 5677147209e8..dd57d05b5dcc 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -184,10 +184,8 @@ struct atm_mpoa_qos *atm_mpoa_add_qos(__be32 dst_ip, struct atm_qos *qos)
}
entry = kmalloc(sizeof(struct atm_mpoa_qos), GFP_KERNEL);
- if (entry == NULL) {
- pr_info("mpoa: out of memory\n");
+ if (!entry)
return entry;
- }
entry->ipaddr = dst_ip;
entry->qos = *qos;
@@ -473,10 +471,8 @@ static const uint8_t *copy_macs(struct mpoa_client *mpc,
kfree(mpc->mps_macs);
mpc->number_of_mps_macs = 0;
mpc->mps_macs = kmalloc(num_macs * ETH_ALEN, GFP_KERNEL);
- if (mpc->mps_macs == NULL) {
- pr_info("(%s) out of mem\n", mpc->dev->name);
+ if (!mpc->mps_macs)
return NULL;
- }
}
ether_addr_copy(mpc->mps_macs, router_mac);
tlvs += 20; if (device_type == MPS_AND_MPC) tlvs += 20;
diff --git a/net/atm/mpoa_caches.c b/net/atm/mpoa_caches.c
index 4ccaa16b1eb1..7495b42d59eb 100644
--- a/net/atm/mpoa_caches.c
+++ b/net/atm/mpoa_caches.c
@@ -98,10 +98,8 @@ static in_cache_entry *in_cache_add_entry(__be32 dst_ip,
{
in_cache_entry *entry = kzalloc(sizeof(in_cache_entry), GFP_KERNEL);
- if (entry == NULL) {
- pr_info("mpoa: mpoa_caches.c: new_in_cache_entry: out of memory\n");
+ if (!entry)
return NULL;
- }
dprintk("adding an ingress entry, ip = %pI4\n", &dst_ip);
@@ -460,10 +458,8 @@ static eg_cache_entry *eg_cache_add_entry(struct k_message *msg,
{
eg_cache_entry *entry = kzalloc(sizeof(eg_cache_entry), GFP_KERNEL);
- if (entry == NULL) {
- pr_info("out of memory\n");
+ if (!entry)
return NULL;
- }
dprintk("adding an egress entry, ip = %pI4, this should be our IP\n",
&msg->content.eg_info.eg_dst_ip);
--
2.14.2
^ permalink raw reply related
* [PATCH 2/3] net/atm: Improve a size determination in 12 functions
From: SF Markus Elfring @ 2017-10-09 20:50 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, Andrew Morton, Augusto Mecking Caringi,
Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
Hans Liljestrand, Jarod Wilson, Johannes Berg, Kees Cook,
Mitchell Blank Jr, Roopa Prabhu
Cc: LKML, kernel-janitors
In-Reply-To: <10a92558-1105-b947-86a2-6ac763cca36d@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 9 Oct 2017 22:00:19 +0200
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/atm/addr.c | 2 +-
net/atm/br2684.c | 2 +-
net/atm/clip.c | 4 ++--
net/atm/lec.c | 6 +++---
net/atm/mpc.c | 4 ++--
net/atm/mpoa_caches.c | 4 ++--
net/atm/signaling.c | 2 +-
7 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/atm/addr.c b/net/atm/addr.c
index dcda35c66f15..048e6d192818 100644
--- a/net/atm/addr.c
+++ b/net/atm/addr.c
@@ -86,7 +86,7 @@ int atm_add_addr(struct atm_dev *dev, const struct sockaddr_atmsvc *addr,
return -EEXIST;
}
}
- this = kmalloc(sizeof(struct atm_dev_addr), GFP_ATOMIC);
+ this = kmalloc(sizeof(*this), GFP_ATOMIC);
if (!this) {
spin_unlock_irqrestore(&dev->lock, flags);
return -ENOMEM;
diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index 4e111196f902..f5b601c01f38 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -538,7 +538,7 @@ static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
if (copy_from_user(&be, arg, sizeof be))
return -EFAULT;
- brvcc = kzalloc(sizeof(struct br2684_vcc), GFP_KERNEL);
+ brvcc = kzalloc(sizeof(*brvcc), GFP_KERNEL);
if (!brvcc)
return -ENOMEM;
/*
diff --git a/net/atm/clip.c b/net/atm/clip.c
index 65f706e4344c..041d519b8771 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -60,7 +60,7 @@ static int to_atmarpd(enum atmarp_ctrl_type type, int itf, __be32 ip)
skb = alloc_skb(sizeof(struct atmarp_ctrl), GFP_ATOMIC);
if (!skb)
return -ENOMEM;
- ctrl = skb_put(skb, sizeof(struct atmarp_ctrl));
+ ctrl = skb_put(skb, sizeof(*ctrl));
ctrl->type = type;
ctrl->itf_num = itf;
ctrl->ip = ip;
@@ -418,7 +418,7 @@ static int clip_mkip(struct atm_vcc *vcc, int timeout)
if (!vcc->push)
return -EBADFD;
- clip_vcc = kmalloc(sizeof(struct clip_vcc), GFP_KERNEL);
+ clip_vcc = kmalloc(sizeof(*clip_vcc), GFP_KERNEL);
if (!clip_vcc)
return -ENOMEM;
pr_debug("%p vcc %p\n", clip_vcc, vcc);
diff --git a/net/atm/lec.c b/net/atm/lec.c
index f5be0b931978..74a794602412 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -690,7 +690,7 @@ static int lec_vcc_attach(struct atm_vcc *vcc, void __user *arg)
if (ioc_data.dev_num < 0 || ioc_data.dev_num >= MAX_LEC_ITF ||
!dev_lec[ioc_data.dev_num])
return -EINVAL;
- vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+ vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
if (!vpriv)
return -ENOMEM;
vpriv->xoff = 0;
@@ -1552,7 +1552,7 @@ static struct lec_arp_table *make_entry(struct lec_priv *priv,
{
struct lec_arp_table *to_return;
- to_return = kzalloc(sizeof(struct lec_arp_table), GFP_ATOMIC);
+ to_return = kzalloc(sizeof(*to_return), GFP_ATOMIC);
if (!to_return)
return NULL;
@@ -2155,7 +2155,7 @@ static int lec_mcast_make(struct lec_priv *priv, struct atm_vcc *vcc)
struct lec_vcc_priv *vpriv;
int err = 0;
- vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+ vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
if (!vpriv)
return -ENOMEM;
vpriv->xoff = 0;
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index dd57d05b5dcc..d6729d797107 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -183,7 +183,7 @@ struct atm_mpoa_qos *atm_mpoa_add_qos(__be32 dst_ip, struct atm_qos *qos)
return entry;
}
- entry = kmalloc(sizeof(struct atm_mpoa_qos), GFP_KERNEL);
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
return entry;
@@ -279,7 +279,7 @@ static struct mpoa_client *alloc_mpc(void)
{
struct mpoa_client *mpc;
- mpc = kzalloc(sizeof(struct mpoa_client), GFP_KERNEL);
+ mpc = kzalloc(sizeof(*mpc), GFP_KERNEL);
if (mpc == NULL)
return NULL;
rwlock_init(&mpc->ingress_lock);
diff --git a/net/atm/mpoa_caches.c b/net/atm/mpoa_caches.c
index 7495b42d59eb..23f36e5a20ee 100644
--- a/net/atm/mpoa_caches.c
+++ b/net/atm/mpoa_caches.c
@@ -96,7 +96,7 @@ static in_cache_entry *in_cache_get_by_vcc(struct atm_vcc *vcc,
static in_cache_entry *in_cache_add_entry(__be32 dst_ip,
struct mpoa_client *client)
{
- in_cache_entry *entry = kzalloc(sizeof(in_cache_entry), GFP_KERNEL);
+ in_cache_entry *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
return NULL;
@@ -456,7 +456,7 @@ static void eg_cache_remove_entry(eg_cache_entry *entry,
static eg_cache_entry *eg_cache_add_entry(struct k_message *msg,
struct mpoa_client *client)
{
- eg_cache_entry *entry = kzalloc(sizeof(eg_cache_entry), GFP_KERNEL);
+ eg_cache_entry *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
return NULL;
diff --git a/net/atm/signaling.c b/net/atm/signaling.c
index 0a20f6e953ac..225234f5cb5f 100644
--- a/net/atm/signaling.c
+++ b/net/atm/signaling.c
@@ -150,7 +150,7 @@ void sigd_enq2(struct atm_vcc *vcc, enum atmsvc_msg_type type,
pr_debug("%d (0x%p)\n", (int)type, vcc);
while (!(skb = alloc_skb(sizeof(struct atmsvc_msg), GFP_KERNEL)))
schedule();
- msg = skb_put_zero(skb, sizeof(struct atmsvc_msg));
+ msg = skb_put_zero(skb, sizeof(*msg));
msg->type = type;
*(struct atm_vcc **) &msg->vcc = vcc;
*(struct atm_vcc **) &msg->listen_vcc = listen_vcc;
--
2.14.2
^ permalink raw reply related
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