* Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.
From: James Hogan @ 2017-11-30 22:53 UTC (permalink / raw)
To: David Daney
Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, ralf-6z/3iImG2C8G8FEW9MqTrA,
netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, Rob Herring,
Mark Rutland, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Steven J. Hill, devicetree-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn,
Florian Fainelli, Carlos Munoz
In-Reply-To: <20171129005540.28829-4-david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 14219 bytes --]
On Tue, Nov 28, 2017 at 04:55:35PM -0800, David Daney wrote:
> From: Carlos Munoz <cmunoz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>
> Add a global resource manager to manage tagged pointers within
> bootmem allocated memory. This is used by various functional
> blocks in the Octeon core like the FPA, Ethernet nexus, etc.
>
> Signed-off-by: Carlos Munoz <cmunoz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Steven J. Hill <Steven.Hill-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
> arch/mips/cavium-octeon/Makefile | 3 +-
> arch/mips/cavium-octeon/resource-mgr.c | 371 +++++++++++++++++++++++++++++++++
> arch/mips/include/asm/octeon/octeon.h | 18 ++
> 3 files changed, 391 insertions(+), 1 deletion(-)
> create mode 100644 arch/mips/cavium-octeon/resource-mgr.c
>
> diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
> index 7c02e542959a..0a299ab8719f 100644
> --- a/arch/mips/cavium-octeon/Makefile
> +++ b/arch/mips/cavium-octeon/Makefile
> @@ -9,7 +9,8 @@
> # Copyright (C) 2005-2009 Cavium Networks
> #
>
> -obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o
> +obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o \
> + resource-mgr.o
Maybe put that on a separate line like below.
> obj-y += dma-octeon.o
> obj-y += octeon-memcpy.o
> obj-y += executive/
> diff --git a/arch/mips/cavium-octeon/resource-mgr.c b/arch/mips/cavium-octeon/resource-mgr.c
> new file mode 100644
> index 000000000000..ca25fa953402
> --- /dev/null
> +++ b/arch/mips/cavium-octeon/resource-mgr.c
> @@ -0,0 +1,371 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Resource manager for Octeon.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2017 Cavium, Inc.
> + */
> +#include <linux/module.h>
> +
> +#include <asm/octeon/octeon.h>
> +#include <asm/octeon/cvmx-bootmem.h>
> +
> +#define RESOURCE_MGR_BLOCK_NAME "cvmx-global-resources"
> +#define MAX_RESOURCES 128
> +#define INST_AVAILABLE -88
> +#define OWNER 0xbadc0de
> +
> +struct global_resource_entry {
> + struct global_resource_tag tag;
> + u64 phys_addr;
> + u64 size;
> +};
> +
> +struct global_resources {
> +#ifdef __LITTLE_ENDIAN_BITFIELD
> + u32 rlock;
> + u32 pad;
> +#else
> + u32 pad;
> + u32 rlock;
> +#endif
> + u64 entry_cnt;
> + struct global_resource_entry resource_entry[];
> +};
> +
> +static struct global_resources *res_mgr_info;
> +
> +
> +/*
> + * The resource manager interacts with software running outside of the
> + * Linux kernel, which necessitates locking to maintain data structure
> + * consistency. These custom locking functions implement the locking
> + * protocol, and cannot be replaced by kernel locking functions that
> + * may use different in-memory structures.
> + */
> +
> +static void res_mgr_lock(void)
> +{
> + unsigned int tmp;
> + u64 lock = (u64)&res_mgr_info->rlock;
presumably this could be a u32 *, avoid the cast to u64, and still work
just fine below.
> +
> + __asm__ __volatile__(
> + ".set noreorder\n"
> + "1: ll %[tmp], 0(%[addr])\n"
> + " bnez %[tmp], 1b\n"
> + " li %[tmp], 1\n"
I believe the convention for .S files is for instructions in branch
delay slots to be indented an additional space for readability. Maybe
that would be worthwhile here.
> + " sc %[tmp], 0(%[addr])\n"
> + " beqz %[tmp], 1b\n"
> + " nop\n"
and here also.
> + ".set reorder\n" :
nit: strictly speaking there's no need for \n on the last line.
> + [tmp] "=&r"(tmp) :
> + [addr] "r"(lock) :
> + "memory");
minor style thing: its far more common to have : at the beginning of the
line rather than the end.
> +}
> +
> +static void res_mgr_unlock(void)
> +{
> + u64 lock = (u64)&res_mgr_info->rlock;
same again
> +
> + /* Wait until all resource operations finish before unlocking. */
> + mb();
> + __asm__ __volatile__(
> + "sw $0, 0(%[addr])\n" : :
> + [addr] "r"(lock) :
> + "memory");
> +
> + /* Force a write buffer flush. */
> + mb();
> +}
> +
> +static int res_mgr_find_resource(struct global_resource_tag tag)
> +{
> + struct global_resource_entry *res_entry;
> + int i;
> +
> + for (i = 0; i < res_mgr_info->entry_cnt; i++) {
> + res_entry = &res_mgr_info->resource_entry[i];
> + if (res_entry->tag.lo == tag.lo && res_entry->tag.hi == tag.hi)
> + return i;
> + }
> + return -1;
> +}
> +
> +/**
> + * res_mgr_create_resource - Create a resource.
> + * @tag: Identifies the resource.
> + * @inst_cnt: Number of resource instances to create.
> + *
> + * Returns 0 if the source was created successfully.
> + * Returns <0 for error codes.
Only -1 seems to be returned. Is it worth returning some standard Linux
error codes instead?
> + */
> +int res_mgr_create_resource(struct global_resource_tag tag, int inst_cnt)
> +{
> + struct global_resource_entry *res_entry;
> + u64 size;
> + u64 *res_addr;
> + int res_index, i, rc = 0;
> +
> + res_mgr_lock();
> +
> + /* Make sure resource doesn't already exist. */
> + res_index = res_mgr_find_resource(tag);
> + if (res_index >= 0) {
> + rc = -1;
> + goto err;
> + }
> +
> + if (res_mgr_info->entry_cnt >= MAX_RESOURCES) {
> + pr_err("Resource max limit reached, not created\n");
> + rc = -1;
> + goto err;
> + }
> +
> + /*
> + * Each instance is kept in an array of u64s. The first array element
> + * holds the number of allocated instances.
> + */
> + size = sizeof(u64) * (inst_cnt + 1);
> + res_addr = cvmx_bootmem_alloc_range(size, CVMX_CACHE_LINE_SIZE, 0, 0);
> + if (!res_addr) {
> + pr_err("Failed to allocate resource. not created\n");
> + rc = -1;
> + goto err;
> + }
> +
> + /* Initialize the newly created resource. */
> + *res_addr = inst_cnt;
> + for (i = 1; i < inst_cnt + 1; i++)
or "i <= inst_cnt"?
> + *(res_addr + i) = INST_AVAILABLE;
Nit: IMO res_addr[i] is marginally more readable
> +
> + res_index = res_mgr_info->entry_cnt;
> + res_entry = &res_mgr_info->resource_entry[res_index];
> + res_entry->tag.lo = tag.lo;
> + res_entry->tag.hi = tag.hi;
or res_entry->tag = tag;?
> + res_entry->phys_addr = virt_to_phys(res_addr);
> + res_entry->size = size;
> + res_mgr_info->entry_cnt++;
> +
> +err:
> + res_mgr_unlock();
> +
> + return rc;
> +}
> +EXPORT_SYMBOL(res_mgr_create_resource);
> +
> +/**
> + * res_mgr_alloc_range - Allocate a range of resource instances.
I don't know how strict kerndoc is on this, but I think it should be
res_mgr_alloc_range() here. Same elsewhere.
> + * @tag: Identifies the resource.
> + * @req_inst: Requested start of instance range to allocate.
> + * Range instances are guaranteed to be sequential
> + * (-1 for don't care).
> + * @req_cnt: Number of instances to allocate.
> + * @use_last_avail: Set to request the last available instance.
> + * @inst: Updated with the allocated instances.
> + *
> + * Returns 0 if the source was created successfully.
> + * Returns <0 for error codes.
> + */
> +int res_mgr_alloc_range(struct global_resource_tag tag, int req_inst,
> + int req_cnt, bool use_last_avail, int *inst)
> +{
> + struct global_resource_entry *res_entry;
> + int res_index;
> + u64 *res_addr;
> + u64 inst_cnt;
> + int alloc_cnt, i, rc = -1;
> +
> + /* Start with no instances allocated. */
> + for (i = 0; i < req_cnt; i++)
> + inst[i] = INST_AVAILABLE;
> +
> + res_mgr_lock();
> +
> + /* Find the resource. */
> + res_index = res_mgr_find_resource(tag);
> + if (res_index < 0) {
> + pr_err("Resource not found, can't allocate instance\n");
> + goto err;
> + }
> +
> + /* Get resource data. */
> + res_entry = &res_mgr_info->resource_entry[res_index];
> + res_addr = phys_to_virt(res_entry->phys_addr);
> + inst_cnt = *res_addr;
> +
> + /* Allocate the requested instances. */
> + if (req_inst >= 0) {
> + /* Specific instance range requested. */
> + if (req_inst + req_cnt >= inst_cnt) {
> + pr_err("Requested instance out of range\n");
> + goto err;
> + }
> +
> + for (i = 0; i < req_cnt; i++) {
> + if (*(res_addr + req_inst + 1 + i) == INST_AVAILABLE)
> + inst[i] = req_inst + i;
> + else {
braces on all branches if on any.
> + inst[0] = INST_AVAILABLE;
> + break;
> + }
> + }
> + } else if (use_last_avail) {
> + /* Last available instance requested. */
> + alloc_cnt = 0;
> + for (i = inst_cnt; i > 0; i--) {
> + if (*(res_addr + i) == INST_AVAILABLE) {
> + /*
> + * Instance off by 1 (first element holds the
> + * count).
> + */
> + inst[alloc_cnt] = i - 1;
> +
> + alloc_cnt++;
> + if (alloc_cnt == req_cnt)
> + break;
> + }
> + }
> +
> + if (i == 0)
> + inst[0] = INST_AVAILABLE;
> + } else {
> + /* Next available instance requested. */
> + alloc_cnt = 0;
> + for (i = 1; i <= inst_cnt; i++) {
> + if (*(res_addr + i) == INST_AVAILABLE) {
> + /*
> + * Instance off by 1 (first element holds the
> + * count).
> + */
> + inst[alloc_cnt] = i - 1;
> +
> + alloc_cnt++;
> + if (alloc_cnt == req_cnt)
> + break;
> + }
> + }
> +
> + if (i > inst_cnt)
> + inst[0] = INST_AVAILABLE;
> + }
> +
> + if (inst[0] != INST_AVAILABLE) {
> + for (i = 0; i < req_cnt; i++)
> + *(res_addr + inst[i] + 1) = OWNER;
> + rc = 0;
> + }
> +
> +err:
> + res_mgr_unlock();
> +
> + return rc;
> +}
> +EXPORT_SYMBOL(res_mgr_alloc_range);
> +
> +/**
> + * res_mgr_alloc - Allocate a resource instance.
> + * @tag: Identifies the resource.
> + * @req_inst: Requested instance to allocate (-1 for don't care).
> + * @use_last_avail: Set to request the last available instance.
> + *
> + * Returns: Allocated resource instance if successful.
> + * Returns <0 for error codes.
> + */
> +int res_mgr_alloc(struct global_resource_tag tag, int req_inst, bool use_last_avail)
> +{
> + int inst, rc;
> +
> + rc = res_mgr_alloc_range(tag, req_inst, 1, use_last_avail, &inst);
> + if (!rc)
> + return inst;
> + return rc;
> +}
> +EXPORT_SYMBOL(res_mgr_alloc);
> +
> +/**
> + * res_mgr_free_range - Free a resource instance range.
> + * @tag: Identifies the resource.
> + * @req_inst: Requested instance to free.
the parameter is called inst.
Other than these minor / style comments, it doesn't look unreasonable to
me.
Cheers
James
> + * @req_cnt: Number of instances to free.
> + */
> +void res_mgr_free_range(struct global_resource_tag tag, const int *inst, int req_cnt)
> +{
> + struct global_resource_entry *res_entry;
> + int res_index, i;
> + u64 *res_addr;
> +
> + res_mgr_lock();
> +
> + /* Find the resource. */
> + res_index = res_mgr_find_resource(tag);
> + if (res_index < 0) {
> + pr_err("Resource not found, can't free instance\n");
> + goto err;
> + }
> +
> + /* Get the resource data. */
> + res_entry = &res_mgr_info->resource_entry[res_index];
> + res_addr = phys_to_virt(res_entry->phys_addr);
> +
> + /* Free the resource instances. */
> + for (i = 0; i < req_cnt; i++) {
> + /* Instance off by 1 (first element holds the count). */
> + *(res_addr + inst[i] + 1) = INST_AVAILABLE;
> + }
> +
> +err:
> + res_mgr_unlock();
> +}
> +EXPORT_SYMBOL(res_mgr_free_range);
> +
> +/**
> + * res_mgr_free - Free a resource instance.
> + * @tag: Identifies the resource.
> + * @req_inst: Requested instance to free.
> + */
> +void res_mgr_free(struct global_resource_tag tag, int inst)
> +{
> + res_mgr_free_range(tag, &inst, 1);
> +}
> +EXPORT_SYMBOL(res_mgr_free);
> +
> +static int __init res_mgr_init(void)
> +{
> + struct cvmx_bootmem_named_block_desc *block;
> + int block_size;
> + u64 addr;
> +
> + cvmx_bootmem_lock();
> +
> + /* Search for the resource manager data in boot memory. */
> + block = cvmx_bootmem_phy_named_block_find(RESOURCE_MGR_BLOCK_NAME, CVMX_BOOTMEM_FLAG_NO_LOCKING);
> + if (block) {
> + /* Found. */
> + res_mgr_info = phys_to_virt(block->base_addr);
> + } else {
> + /* Create it. */
> + block_size = sizeof(struct global_resources) +
> + sizeof(struct global_resource_entry) * MAX_RESOURCES;
> + addr = cvmx_bootmem_phy_named_block_alloc(block_size, 0, 0,
> + CVMX_CACHE_LINE_SIZE, RESOURCE_MGR_BLOCK_NAME,
> + CVMX_BOOTMEM_FLAG_NO_LOCKING);
> + if (!addr) {
> + pr_err("Failed to allocate name block %s\n",
> + RESOURCE_MGR_BLOCK_NAME);
> + } else {
> + res_mgr_info = phys_to_virt(addr);
> + memset(res_mgr_info, 0, block_size);
> + }
> + }
> +
> + cvmx_bootmem_unlock();
> +
> + return 0;
> +}
> +device_initcall(res_mgr_init);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Cavium, Inc. Octeon resource manager");
> diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h
> index 92a17d67c1fa..0411efdb465c 100644
> --- a/arch/mips/include/asm/octeon/octeon.h
> +++ b/arch/mips/include/asm/octeon/octeon.h
> @@ -346,6 +346,24 @@ void octeon_mult_restore3_end(void);
> void octeon_mult_restore2(void);
> void octeon_mult_restore2_end(void);
>
> +/*
> + * This definition must be kept in sync with the one in
> + * cvmx-global-resources.c
> + */
> +struct global_resource_tag {
> + uint64_t lo;
> + uint64_t hi;
> +};
> +
> +void res_mgr_free(struct global_resource_tag tag, int inst);
> +void res_mgr_free_range(struct global_resource_tag tag, const int *inst,
> + int req_cnt);
> +int res_mgr_alloc(struct global_resource_tag tag, int req_inst,
> + bool use_last_avail);
> +int res_mgr_alloc_range(struct global_resource_tag tag, int req_inst,
> + int req_cnt, bool use_last_avail, int *inst);
> +int res_mgr_create_resource(struct global_resource_tag tag, int inst_cnt);
> +
> /**
> * Read a 32bit value from the Octeon NPI register space
> *
> --
> 2.14.3
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH net-next resubmit 2/2] net: phy: remove generic settings for callbacks config_aneg and read_status from drivers
From: Heiner Kallweit @ 2017-11-30 22:47 UTC (permalink / raw)
To: David Miller, Florian Fainelli, Andrew Lunn; +Cc: netdev@vger.kernel.org
In-Reply-To: <e9350088-76f9-be2c-e2ac-b3860d96e095@gmail.com>
Remove generic settings for callbacks config_aneg and read_status
from drivers.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/amd.c | 2 --
drivers/net/phy/at803x.c | 6 ------
drivers/net/phy/bcm-cygnus.c | 2 --
drivers/net/phy/bcm63xx.c | 4 ----
drivers/net/phy/bcm7xxx.c | 6 ------
drivers/net/phy/broadcom.c | 30 ------------------------------
drivers/net/phy/cicada.c | 4 ----
drivers/net/phy/davicom.c | 5 -----
drivers/net/phy/dp83640.c | 2 --
drivers/net/phy/dp83822.c | 2 --
drivers/net/phy/dp83848.c | 2 --
drivers/net/phy/dp83867.c | 2 --
drivers/net/phy/icplus.c | 4 ----
drivers/net/phy/intel-xway.c | 12 ------------
drivers/net/phy/lxt.c | 5 -----
drivers/net/phy/marvell.c | 9 ---------
drivers/net/phy/meson-gxl.c | 2 --
drivers/net/phy/micrel.c | 24 ------------------------
drivers/net/phy/microchip.c | 1 -
drivers/net/phy/national.c | 2 --
drivers/net/phy/phy_device.c | 2 --
drivers/net/phy/qsemi.c | 2 --
drivers/net/phy/realtek.c | 12 ------------
drivers/net/phy/rockchip.c | 1 -
drivers/net/phy/smsc.c | 9 ---------
drivers/net/phy/ste10Xp.c | 4 ----
drivers/net/phy/uPD60620.c | 1 -
drivers/net/phy/vitesse.c | 12 ------------
28 files changed, 169 deletions(-)
diff --git a/drivers/net/phy/amd.c b/drivers/net/phy/amd.c
index 18141c022..6fe5dc920 100644
--- a/drivers/net/phy/amd.c
+++ b/drivers/net/phy/amd.c
@@ -68,8 +68,6 @@ static struct phy_driver am79c_driver[] = { {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = am79c_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = am79c_ack_interrupt,
.config_intr = am79c_config_intr,
} };
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 5f93e6add..de7dd6566 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -408,8 +408,6 @@ static struct phy_driver at803x_driver[] = {
.resume = at803x_resume,
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = at803x_ack_interrupt,
.config_intr = at803x_config_intr,
}, {
@@ -426,8 +424,6 @@ static struct phy_driver at803x_driver[] = {
.resume = at803x_resume,
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = at803x_ack_interrupt,
.config_intr = at803x_config_intr,
}, {
@@ -443,8 +439,6 @@ static struct phy_driver at803x_driver[] = {
.resume = at803x_resume,
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.aneg_done = at803x_aneg_done,
.ack_interrupt = &at803x_ack_interrupt,
.config_intr = &at803x_config_intr,
diff --git a/drivers/net/phy/bcm-cygnus.c b/drivers/net/phy/bcm-cygnus.c
index 3fe8cc5c1..683812983 100644
--- a/drivers/net/phy/bcm-cygnus.c
+++ b/drivers/net/phy/bcm-cygnus.c
@@ -136,8 +136,6 @@ static struct phy_driver bcm_cygnus_phy_driver[] = {
.name = "Broadcom Cygnus PHY",
.features = PHY_GBIT_FEATURES,
.config_init = bcm_cygnus_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
.suspend = genphy_suspend,
diff --git a/drivers/net/phy/bcm63xx.c b/drivers/net/phy/bcm63xx.c
index b0492ef2c..cf1461374 100644
--- a/drivers/net/phy/bcm63xx.c
+++ b/drivers/net/phy/bcm63xx.c
@@ -69,8 +69,6 @@ static struct phy_driver bcm63xx_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_INTERRUPT | PHY_IS_INTERNAL,
.config_init = bcm63xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm63xx_config_intr,
}, {
@@ -81,8 +79,6 @@ static struct phy_driver bcm63xx_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_INTERRUPT | PHY_IS_INTERNAL,
.config_init = bcm63xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm63xx_config_intr,
} };
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 8b33f688a..421feb8f9 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -611,8 +611,6 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
.features = PHY_GBIT_FEATURES, \
.flags = PHY_IS_INTERNAL, \
.config_init = bcm7xxx_28nm_config_init, \
- .config_aneg = genphy_config_aneg, \
- .read_status = genphy_read_status, \
.resume = bcm7xxx_28nm_resume, \
.get_tunable = bcm7xxx_28nm_get_tunable, \
.set_tunable = bcm7xxx_28nm_set_tunable, \
@@ -630,8 +628,6 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
.features = PHY_BASIC_FEATURES, \
.flags = PHY_IS_INTERNAL, \
.config_init = bcm7xxx_28nm_ephy_config_init, \
- .config_aneg = genphy_config_aneg, \
- .read_status = genphy_read_status, \
.resume = bcm7xxx_28nm_ephy_resume, \
.get_sset_count = bcm_phy_get_sset_count, \
.get_strings = bcm_phy_get_strings, \
@@ -647,8 +643,6 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
.features = PHY_BASIC_FEATURES, \
.flags = PHY_IS_INTERNAL, \
.config_init = bcm7xxx_config_init, \
- .config_aneg = genphy_config_aneg, \
- .read_status = genphy_read_status, \
.suspend = bcm7xxx_suspend, \
.resume = bcm7xxx_config_init, \
}
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index d7ed69dea..e008411a3 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -548,8 +548,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -559,8 +557,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -570,8 +566,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -581,8 +575,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -592,8 +584,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -603,8 +593,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -614,8 +602,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -625,8 +611,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = bcm5481_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -636,8 +620,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = bcm5481_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -647,8 +629,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm5482_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = bcm5482_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -658,8 +638,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -669,8 +647,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -680,8 +656,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = bcm54xx_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = bcm_phy_ack_intr,
.config_intr = bcm_phy_config_intr,
}, {
@@ -691,8 +665,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = brcm_fet_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = brcm_fet_ack_interrupt,
.config_intr = brcm_fet_config_intr,
}, {
@@ -702,8 +674,6 @@ static struct phy_driver broadcom_drivers[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = brcm_fet_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = brcm_fet_ack_interrupt,
.config_intr = brcm_fet_config_intr,
} };
diff --git a/drivers/net/phy/cicada.c b/drivers/net/phy/cicada.c
index d339c1afe..c05af00bf 100644
--- a/drivers/net/phy/cicada.c
+++ b/drivers/net/phy/cicada.c
@@ -110,8 +110,6 @@ static struct phy_driver cis820x_driver[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = &cis820x_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &cis820x_ack_interrupt,
.config_intr = &cis820x_config_intr,
}, {
@@ -121,8 +119,6 @@ static struct phy_driver cis820x_driver[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = &cis820x_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &cis820x_ack_interrupt,
.config_intr = &cis820x_config_intr,
} };
diff --git a/drivers/net/phy/davicom.c b/drivers/net/phy/davicom.c
index e28913d9e..5ee99b3b4 100644
--- a/drivers/net/phy/davicom.c
+++ b/drivers/net/phy/davicom.c
@@ -153,7 +153,6 @@ static struct phy_driver dm91xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = dm9161_config_init,
.config_aneg = dm9161_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = dm9161_ack_interrupt,
.config_intr = dm9161_config_intr,
}, {
@@ -164,7 +163,6 @@ static struct phy_driver dm91xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = dm9161_config_init,
.config_aneg = dm9161_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = dm9161_ack_interrupt,
.config_intr = dm9161_config_intr,
}, {
@@ -175,7 +173,6 @@ static struct phy_driver dm91xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = dm9161_config_init,
.config_aneg = dm9161_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = dm9161_ack_interrupt,
.config_intr = dm9161_config_intr,
}, {
@@ -184,8 +181,6 @@ static struct phy_driver dm91xx_driver[] = {
.phy_id_mask = 0x0ffffff0,
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = dm9161_ack_interrupt,
.config_intr = dm9161_config_intr,
} };
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index cbd629822..654f42d00 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1502,8 +1502,6 @@ static struct phy_driver dp83640_driver = {
.probe = dp83640_probe,
.remove = dp83640_remove,
.config_init = dp83640_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = dp83640_ack_interrupt,
.config_intr = dp83640_config_intr,
.ts_info = dp83640_ts_info,
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 14335d14e..6e8a2a4f3 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -325,8 +325,6 @@ static struct phy_driver dp83822_driver[] = {
.set_wol = dp83822_set_wol,
.ack_interrupt = dp83822_ack_interrupt,
.config_intr = dp83822_config_intr,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.suspend = dp83822_suspend,
.resume = dp83822_resume,
},
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 3966d43c5..cd09c3af2 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -95,8 +95,6 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
.config_init = genphy_config_init, \
.suspend = genphy_suspend, \
.resume = genphy_resume, \
- .config_aneg = genphy_config_aneg, \
- .read_status = genphy_read_status, \
\
/* IRQ related */ \
.ack_interrupt = dp83848_ack_interrupt, \
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index c1ab976cc..ab58224f8 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -324,8 +324,6 @@ static struct phy_driver dp83867_driver[] = {
.ack_interrupt = dp83867_ack_interrupt,
.config_intr = dp83867_config_intr,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
},
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
index 567280a72..791587a49 100644
--- a/drivers/net/phy/icplus.c
+++ b/drivers/net/phy/icplus.c
@@ -227,8 +227,6 @@ static struct phy_driver icplus_driver[] = {
.phy_id_mask = 0x0ffffff0,
.features = PHY_GBIT_FEATURES,
.config_init = &ip1001_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -239,8 +237,6 @@ static struct phy_driver icplus_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.ack_interrupt = ip101a_g_ack_interrupt,
.config_init = &ip101a_g_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
} };
diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index 55f8c52dd..a11f80cb5 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -243,7 +243,6 @@ static struct phy_driver xway_gphy[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
@@ -257,7 +256,6 @@ static struct phy_driver xway_gphy[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
@@ -271,7 +269,6 @@ static struct phy_driver xway_gphy[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
@@ -285,7 +282,6 @@ static struct phy_driver xway_gphy[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
@@ -298,8 +294,6 @@ static struct phy_driver xway_gphy[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
@@ -312,8 +306,6 @@ static struct phy_driver xway_gphy[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
@@ -326,8 +318,6 @@ static struct phy_driver xway_gphy[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
@@ -340,8 +330,6 @@ static struct phy_driver xway_gphy[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = xway_gphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = xway_gphy_ack_interrupt,
.did_interrupt = xway_gphy_did_interrupt,
.config_intr = xway_gphy_config_intr,
diff --git a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c
index 09d215177..c14b254b2 100644
--- a/drivers/net/phy/lxt.c
+++ b/drivers/net/phy/lxt.c
@@ -259,8 +259,6 @@ static struct phy_driver lxt97x_driver[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = lxt970_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = lxt970_ack_interrupt,
.config_intr = lxt970_config_intr,
}, {
@@ -269,8 +267,6 @@ static struct phy_driver lxt97x_driver[] = {
.phy_id_mask = 0xfffffff0,
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = lxt971_ack_interrupt,
.config_intr = lxt971_config_intr,
}, {
@@ -290,7 +286,6 @@ static struct phy_driver lxt97x_driver[] = {
.flags = 0,
.probe = lxt973_probe,
.config_aneg = lxt973_config_aneg,
- .read_status = genphy_read_status,
} };
module_phy_driver(lxt97x_driver);
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 4d02b27df..6dbb0f4c3 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -1958,7 +1958,6 @@ static struct phy_driver marvell_drivers[] = {
.probe = marvell_probe,
.config_init = &marvell_config_init,
.config_aneg = &m88e1101_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
@@ -1976,7 +1975,6 @@ static struct phy_driver marvell_drivers[] = {
.probe = marvell_probe,
.config_init = &m88e1111_config_init,
.config_aneg = &marvell_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
@@ -2012,7 +2010,6 @@ static struct phy_driver marvell_drivers[] = {
.probe = marvell_probe,
.config_init = &m88e1118_config_init,
.config_aneg = &m88e1118_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
@@ -2070,7 +2067,6 @@ static struct phy_driver marvell_drivers[] = {
.probe = marvell_probe,
.config_init = &m88e1145_config_init,
.config_aneg = &marvell_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
@@ -2088,7 +2084,6 @@ static struct phy_driver marvell_drivers[] = {
.probe = marvell_probe,
.config_init = &m88e1149_config_init,
.config_aneg = &m88e1118_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
@@ -2106,7 +2101,6 @@ static struct phy_driver marvell_drivers[] = {
.probe = marvell_probe,
.config_init = &m88e1111_config_init,
.config_aneg = &marvell_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
@@ -2123,8 +2117,6 @@ static struct phy_driver marvell_drivers[] = {
.flags = PHY_HAS_INTERRUPT,
.probe = marvell_probe,
.config_init = &m88e1116r_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
@@ -2200,7 +2192,6 @@ static struct phy_driver marvell_drivers[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.probe = marvell_probe,
- .config_aneg = &genphy_config_aneg,
.config_init = &m88e3016_config_init,
.aneg_done = &marvell_aneg_done,
.read_status = &marvell_read_status,
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 1ea69b758..401e3234b 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -58,9 +58,7 @@ static struct phy_driver meson_gxl_phy[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_IS_INTERNAL,
.config_init = meson_gxl_config_init,
- .config_aneg = genphy_config_aneg,
.aneg_done = genphy_aneg_done,
- .read_status = genphy_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
},
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index fdb43dd9b..cf1c5b1ed 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -799,8 +799,6 @@ static struct phy_driver ksphy_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.driver_data = &ks8737_type,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
@@ -814,8 +812,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8021_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -832,8 +828,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8021_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -851,7 +845,6 @@ static struct phy_driver ksphy_driver[] = {
.probe = kszphy_probe,
.config_init = ksz8041_config_init,
.config_aneg = ksz8041_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -868,8 +861,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8041_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -886,8 +877,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8051_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -904,8 +893,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8041_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -922,8 +909,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8081_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -938,8 +923,6 @@ static struct phy_driver ksphy_driver[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
@@ -953,8 +936,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz9021_type,
.probe = kszphy_probe,
.config_init = ksz9021_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.get_sset_count = kszphy_get_sset_count,
@@ -973,7 +954,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz9021_type,
.probe = kszphy_probe,
.config_init = ksz9031_config_init,
- .config_aneg = genphy_config_aneg,
.read_status = ksz9031_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
@@ -998,8 +978,6 @@ static struct phy_driver ksphy_driver[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -1019,8 +997,6 @@ static struct phy_driver ksphy_driver[] = {
.name = "Microchip KSZ9477",
.features = PHY_GBIT_FEATURES,
.config_init = kszphy_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
} };
diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index 37ee856c7..0f293ef28 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -153,7 +153,6 @@ static struct phy_driver microchip_phy_driver[] = {
.config_init = genphy_config_init,
.config_aneg = lan88xx_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = lan88xx_phy_ack_interrupt,
.config_intr = lan88xx_phy_config_intr,
diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
index 2addf1d3f..2b1e33696 100644
--- a/drivers/net/phy/national.c
+++ b/drivers/net/phy/national.c
@@ -136,8 +136,6 @@ static struct phy_driver dp83865_driver[] = { {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = ns_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = ns_ack_interrupt,
.config_intr = ns_config_intr,
} };
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 67f25ac29..8154fb706 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1907,9 +1907,7 @@ static struct phy_driver genphy_driver = {
.features = PHY_GBIT_FEATURES | SUPPORTED_MII |
SUPPORTED_AUI | SUPPORTED_FIBRE |
SUPPORTED_BNC,
- .config_aneg = genphy_config_aneg,
.aneg_done = genphy_aneg_done,
- .read_status = genphy_read_status,
.suspend = genphy_suspend,
.resume = genphy_resume,
.set_loopback = genphy_loopback,
diff --git a/drivers/net/phy/qsemi.c b/drivers/net/phy/qsemi.c
index dbef8002b..889a4dce1 100644
--- a/drivers/net/phy/qsemi.c
+++ b/drivers/net/phy/qsemi.c
@@ -118,8 +118,6 @@ static struct phy_driver qs6612_driver[] = { {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = qs6612_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = qs6612_ack_interrupt,
.config_intr = qs6612_config_intr,
} };
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index eda0a6e86..82efbf670 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -159,16 +159,12 @@ static struct phy_driver realtek_drvs[] = {
.phy_id_mask = 0x0000ffff,
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
}, {
.phy_id = 0x001cc816,
.name = "RTL8201F 10/100Mbps Ethernet",
.phy_id_mask = 0x001fffff,
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &rtl8201_ack_interrupt,
.config_intr = &rtl8201_config_intr,
.suspend = genphy_suspend,
@@ -179,8 +175,6 @@ static struct phy_driver realtek_drvs[] = {
.phy_id_mask = 0x001fffff,
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &rtl821x_ack_interrupt,
.config_intr = &rtl8211b_config_intr,
}, {
@@ -189,8 +183,6 @@ static struct phy_driver realtek_drvs[] = {
.phy_id_mask = 0x001fffff,
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = rtl821x_ack_interrupt,
.config_intr = rtl8211e_config_intr,
.suspend = genphy_suspend,
@@ -201,8 +193,6 @@ static struct phy_driver realtek_drvs[] = {
.phy_id_mask = 0x001fffff,
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &rtl821x_ack_interrupt,
.config_intr = &rtl8211e_config_intr,
.suspend = genphy_suspend,
@@ -213,9 +203,7 @@ static struct phy_driver realtek_drvs[] = {
.phy_id_mask = 0x001fffff,
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_aneg = &genphy_config_aneg,
.config_init = &rtl8211f_config_init,
- .read_status = &genphy_read_status,
.ack_interrupt = &rtl8211f_ack_interrupt,
.config_intr = &rtl8211f_config_intr,
.suspend = genphy_suspend,
diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c
index c092af137..f1da70b9b 100644
--- a/drivers/net/phy/rockchip.c
+++ b/drivers/net/phy/rockchip.c
@@ -213,7 +213,6 @@ static struct phy_driver rockchip_phy_driver[] = {
.soft_reset = genphy_soft_reset,
.config_init = rockchip_integrated_phy_config_init,
.config_aneg = rockchip_config_aneg,
- .read_status = genphy_read_status,
.suspend = genphy_suspend,
.resume = rockchip_phy_resume,
},
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 2306bfae0..a1961ba87 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -227,8 +227,6 @@ static struct phy_driver smsc_phy_driver[] = {
.probe = smsc_phy_probe,
/* basic functions */
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.config_init = smsc_phy_config_init,
.soft_reset = smsc_phy_reset,
@@ -249,8 +247,6 @@ static struct phy_driver smsc_phy_driver[] = {
.probe = smsc_phy_probe,
/* basic functions */
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.config_init = smsc_phy_config_init,
.soft_reset = smsc_phy_reset,
@@ -276,7 +272,6 @@ static struct phy_driver smsc_phy_driver[] = {
.probe = smsc_phy_probe,
/* basic functions */
- .config_aneg = genphy_config_aneg,
.read_status = lan87xx_read_status,
.config_init = smsc_phy_config_init,
.soft_reset = smsc_phy_reset,
@@ -303,8 +298,6 @@ static struct phy_driver smsc_phy_driver[] = {
.probe = smsc_phy_probe,
/* basic functions */
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.config_init = lan911x_config_init,
/* IRQ related */
@@ -324,7 +317,6 @@ static struct phy_driver smsc_phy_driver[] = {
.probe = smsc_phy_probe,
/* basic functions */
- .config_aneg = genphy_config_aneg,
.read_status = lan87xx_read_status,
.config_init = smsc_phy_config_init,
.soft_reset = smsc_phy_reset,
@@ -351,7 +343,6 @@ static struct phy_driver smsc_phy_driver[] = {
.probe = smsc_phy_probe,
/* basic functions */
- .config_aneg = genphy_config_aneg,
.read_status = lan87xx_read_status,
.config_init = smsc_phy_config_init,
.soft_reset = smsc_phy_reset,
diff --git a/drivers/net/phy/ste10Xp.c b/drivers/net/phy/ste10Xp.c
index d00cfb645..fbd548a1a 100644
--- a/drivers/net/phy/ste10Xp.c
+++ b/drivers/net/phy/ste10Xp.c
@@ -89,8 +89,6 @@ static struct phy_driver ste10xp_pdriver[] = {
.features = PHY_BASIC_FEATURES | SUPPORTED_Pause,
.flags = PHY_HAS_INTERRUPT,
.config_init = ste10Xp_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = ste10Xp_ack_interrupt,
.config_intr = ste10Xp_config_intr,
.suspend = genphy_suspend,
@@ -102,8 +100,6 @@ static struct phy_driver ste10xp_pdriver[] = {
.features = PHY_BASIC_FEATURES | SUPPORTED_Pause,
.flags = PHY_HAS_INTERRUPT,
.config_init = ste10Xp_config_init,
- .config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
.ack_interrupt = ste10Xp_ack_interrupt,
.config_intr = ste10Xp_config_intr,
.suspend = genphy_suspend,
diff --git a/drivers/net/phy/uPD60620.c b/drivers/net/phy/uPD60620.c
index 96b33475e..55f48ee35 100644
--- a/drivers/net/phy/uPD60620.c
+++ b/drivers/net/phy/uPD60620.c
@@ -95,7 +95,6 @@ static struct phy_driver upd60620_driver[1] = { {
.features = PHY_BASIC_FEATURES,
.flags = 0,
.config_init = upd60620_config_init,
- .config_aneg = genphy_config_aneg,
.read_status = upd60620_read_status,
} };
diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index f78ff0279..d9dd8fbff 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -267,7 +267,6 @@ static struct phy_driver vsc82xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc824x_config_init,
.config_aneg = &vsc82x4_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -278,7 +277,6 @@ static struct phy_driver vsc82xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc824x_config_init,
.config_aneg = &vsc82x4_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -289,7 +287,6 @@ static struct phy_driver vsc82xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc824x_config_init,
.config_aneg = &vsc82x4_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -300,7 +297,6 @@ static struct phy_driver vsc82xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc824x_config_init,
.config_aneg = &vsc82x4_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -311,7 +307,6 @@ static struct phy_driver vsc82xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc824x_config_init,
.config_aneg = &vsc82x4_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -321,8 +316,6 @@ static struct phy_driver vsc82xx_driver[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc8601_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -333,7 +326,6 @@ static struct phy_driver vsc82xx_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc824x_config_init,
.config_aneg = &vsc82x4_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -344,8 +336,6 @@ static struct phy_driver vsc82xx_driver[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc8221_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
}, {
@@ -356,8 +346,6 @@ static struct phy_driver vsc82xx_driver[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init = &vsc8221_config_init,
- .config_aneg = &genphy_config_aneg,
- .read_status = &genphy_read_status,
.ack_interrupt = &vsc824x_ack_interrupt,
.config_intr = &vsc82xx_config_intr,
} };
--
2.15.0
^ permalink raw reply related
* [PATCH net-next resubmit 1/2] net: phy: core: use genphy version of callbacks read_status and config_aneg per default
From: Heiner Kallweit @ 2017-11-30 22:46 UTC (permalink / raw)
To: David Miller, Florian Fainelli, Andrew Lunn; +Cc: netdev@vger.kernel.org
read_status and config_aneg are the only mandatory callbacks and most
of the time the generic implementation is used by drivers.
So make the core fall back to the generic version if a driver doesn't
implement the respective callback.
Also currently the core doesn't seem to verify that drivers implement
the mandatory calls. If a driver doesn't do so we'd just get a NPE.
With this patch this potential issue doesn't exit any longer.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/phy.c | 5 ++++-
include/linux/phy.h | 33 ++++++++++++++++++---------------
2 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 2b1e67bc1..a0e7605dc 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -493,7 +493,10 @@ static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
/* Invalidate LP advertising flags */
phydev->lp_advertising = 0;
- err = phydev->drv->config_aneg(phydev);
+ if (phydev->drv->config_aneg)
+ err = phydev->drv->config_aneg(phydev);
+ else
+ err = genphy_config_aneg(phydev);
if (err < 0)
goto out_unlock;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index dc82a07cb..958b5162a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -497,13 +497,13 @@ struct phy_device {
* flags: A bitfield defining certain other features this PHY
* supports (like interrupts)
*
- * The drivers must implement config_aneg and read_status. All
- * other functions are optional. Note that none of these
- * functions should be called from interrupt time. The goal is
- * for the bus read/write functions to be able to block when the
- * bus transaction is happening, and be freed up by an interrupt
- * (The MPC85xx has this ability, though it is not currently
- * supported in the driver).
+ * All functions are optional. If config_aneg or read_status
+ * are not implemented, the phy core uses the genphy versions.
+ * Note that none of these functions should be called from
+ * interrupt time. The goal is for the bus read/write functions
+ * to be able to block when the bus transaction is happening,
+ * and be freed up by an interrupt (The MPC85xx has this ability,
+ * though it is not currently supported in the driver).
*/
struct phy_driver {
struct mdio_driver_common mdiodrv;
@@ -841,14 +841,6 @@ int phy_aneg_done(struct phy_device *phydev);
int phy_stop_interrupts(struct phy_device *phydev);
int phy_restart_aneg(struct phy_device *phydev);
-static inline int phy_read_status(struct phy_device *phydev)
-{
- if (!phydev->drv)
- return -EIO;
-
- return phydev->drv->read_status(phydev);
-}
-
#define phydev_err(_phydev, format, args...) \
dev_err(&_phydev->mdio.dev, format, ##args)
@@ -890,6 +882,17 @@ int genphy_c45_read_pma(struct phy_device *phydev);
int genphy_c45_pma_setup_forced(struct phy_device *phydev);
int genphy_c45_an_disable_aneg(struct phy_device *phydev);
+static inline int phy_read_status(struct phy_device *phydev)
+{
+ if (!phydev->drv)
+ return -EIO;
+
+ if (phydev->drv->read_status)
+ return phydev->drv->read_status(phydev);
+ else
+ return genphy_read_status(phydev);
+}
+
void phy_driver_unregister(struct phy_driver *drv);
void phy_drivers_unregister(struct phy_driver *drv, int n);
int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 8/8] net: dummy: remove fake SR-IOV functionality
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev
Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski, Phil Sutter, Sabrina Dubroca
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
netdevsim driver seems like a better place for fake SR-IOV
functionality. Remove the code previously added to dummy.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
CC: Phil Sutter <phil@nwl.cc>
CC: Sabrina Dubroca <sd@queasysnail.net>
drivers/net/dummy.c | 215 +---------------------------------------------------
1 file changed, 1 insertion(+), 214 deletions(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 58483af80bdb..30b1c8512049 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -42,48 +42,7 @@
#define DRV_NAME "dummy"
#define DRV_VERSION "1.0"
-#undef pr_fmt
-#define pr_fmt(fmt) DRV_NAME ": " fmt
-
static int numdummies = 1;
-static int num_vfs;
-
-struct vf_data_storage {
- u8 vf_mac[ETH_ALEN];
- u16 pf_vlan; /* When set, guest VLAN config not allowed. */
- u16 pf_qos;
- __be16 vlan_proto;
- u16 min_tx_rate;
- u16 max_tx_rate;
- u8 spoofchk_enabled;
- bool rss_query_enabled;
- u8 trusted;
- int link_state;
-};
-
-struct dummy_priv {
- struct vf_data_storage *vfinfo;
-};
-
-static int dummy_num_vf(struct device *dev)
-{
- return num_vfs;
-}
-
-static struct bus_type dummy_bus = {
- .name = "dummy",
- .num_vf = dummy_num_vf,
-};
-
-static void release_dummy_parent(struct device *dev)
-{
-}
-
-static struct device dummy_parent = {
- .init_name = "dummy",
- .bus = &dummy_bus,
- .release = release_dummy_parent,
-};
/* fake multicast ability */
static void set_multicast_list(struct net_device *dev)
@@ -133,25 +92,10 @@ static netdev_tx_t dummy_xmit(struct sk_buff *skb, struct net_device *dev)
static int dummy_dev_init(struct net_device *dev)
{
- struct dummy_priv *priv = netdev_priv(dev);
-
dev->dstats = netdev_alloc_pcpu_stats(struct pcpu_dstats);
if (!dev->dstats)
return -ENOMEM;
- priv->vfinfo = NULL;
-
- if (!num_vfs)
- return 0;
-
- dev->dev.parent = &dummy_parent;
- priv->vfinfo = kcalloc(num_vfs, sizeof(struct vf_data_storage),
- GFP_KERNEL);
- if (!priv->vfinfo) {
- free_percpu(dev->dstats);
- return -ENOMEM;
- }
-
return 0;
}
@@ -169,117 +113,6 @@ static int dummy_change_carrier(struct net_device *dev, bool new_carrier)
return 0;
}
-static int dummy_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if (!is_valid_ether_addr(mac) || (vf >= num_vfs))
- return -EINVAL;
-
- memcpy(priv->vfinfo[vf].vf_mac, mac, ETH_ALEN);
-
- return 0;
-}
-
-static int dummy_set_vf_vlan(struct net_device *dev, int vf,
- u16 vlan, u8 qos, __be16 vlan_proto)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if ((vf >= num_vfs) || (vlan > 4095) || (qos > 7))
- return -EINVAL;
-
- priv->vfinfo[vf].pf_vlan = vlan;
- priv->vfinfo[vf].pf_qos = qos;
- priv->vfinfo[vf].vlan_proto = vlan_proto;
-
- return 0;
-}
-
-static int dummy_set_vf_rate(struct net_device *dev, int vf, int min, int max)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if (vf >= num_vfs)
- return -EINVAL;
-
- priv->vfinfo[vf].min_tx_rate = min;
- priv->vfinfo[vf].max_tx_rate = max;
-
- return 0;
-}
-
-static int dummy_set_vf_spoofchk(struct net_device *dev, int vf, bool val)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if (vf >= num_vfs)
- return -EINVAL;
-
- priv->vfinfo[vf].spoofchk_enabled = val;
-
- return 0;
-}
-
-static int dummy_set_vf_rss_query_en(struct net_device *dev, int vf, bool val)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if (vf >= num_vfs)
- return -EINVAL;
-
- priv->vfinfo[vf].rss_query_enabled = val;
-
- return 0;
-}
-
-static int dummy_set_vf_trust(struct net_device *dev, int vf, bool val)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if (vf >= num_vfs)
- return -EINVAL;
-
- priv->vfinfo[vf].trusted = val;
-
- return 0;
-}
-
-static int dummy_get_vf_config(struct net_device *dev,
- int vf, struct ifla_vf_info *ivi)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if (vf >= num_vfs)
- return -EINVAL;
-
- ivi->vf = vf;
- memcpy(&ivi->mac, priv->vfinfo[vf].vf_mac, ETH_ALEN);
- ivi->vlan = priv->vfinfo[vf].pf_vlan;
- ivi->qos = priv->vfinfo[vf].pf_qos;
- ivi->spoofchk = priv->vfinfo[vf].spoofchk_enabled;
- ivi->linkstate = priv->vfinfo[vf].link_state;
- ivi->min_tx_rate = priv->vfinfo[vf].min_tx_rate;
- ivi->max_tx_rate = priv->vfinfo[vf].max_tx_rate;
- ivi->rss_query_en = priv->vfinfo[vf].rss_query_enabled;
- ivi->trusted = priv->vfinfo[vf].trusted;
- ivi->vlan_proto = priv->vfinfo[vf].vlan_proto;
-
- return 0;
-}
-
-static int dummy_set_vf_link_state(struct net_device *dev, int vf, int state)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- if (vf >= num_vfs)
- return -EINVAL;
-
- priv->vfinfo[vf].link_state = state;
-
- return 0;
-}
-
static const struct net_device_ops dummy_netdev_ops = {
.ndo_init = dummy_dev_init,
.ndo_uninit = dummy_dev_uninit,
@@ -289,14 +122,6 @@ static const struct net_device_ops dummy_netdev_ops = {
.ndo_set_mac_address = eth_mac_addr,
.ndo_get_stats64 = dummy_get_stats64,
.ndo_change_carrier = dummy_change_carrier,
- .ndo_set_vf_mac = dummy_set_vf_mac,
- .ndo_set_vf_vlan = dummy_set_vf_vlan,
- .ndo_set_vf_rate = dummy_set_vf_rate,
- .ndo_set_vf_spoofchk = dummy_set_vf_spoofchk,
- .ndo_set_vf_trust = dummy_set_vf_trust,
- .ndo_get_vf_config = dummy_get_vf_config,
- .ndo_set_vf_link_state = dummy_set_vf_link_state,
- .ndo_set_vf_rss_query_en = dummy_set_vf_rss_query_en,
};
static void dummy_get_drvinfo(struct net_device *dev,
@@ -323,13 +148,6 @@ static const struct ethtool_ops dummy_ethtool_ops = {
.get_ts_info = dummy_get_ts_info,
};
-static void dummy_free_netdev(struct net_device *dev)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- kfree(priv->vfinfo);
-}
-
static void dummy_setup(struct net_device *dev)
{
ether_setup(dev);
@@ -338,7 +156,6 @@ static void dummy_setup(struct net_device *dev)
dev->netdev_ops = &dummy_netdev_ops;
dev->ethtool_ops = &dummy_ethtool_ops;
dev->needs_free_netdev = true;
- dev->priv_destructor = dummy_free_netdev;
/* Fill in device structure with ethernet-generic values. */
dev->flags |= IFF_NOARP;
@@ -370,7 +187,6 @@ static int dummy_validate(struct nlattr *tb[], struct nlattr *data[],
static struct rtnl_link_ops dummy_link_ops __read_mostly = {
.kind = DRV_NAME,
- .priv_size = sizeof(struct dummy_priv),
.setup = dummy_setup,
.validate = dummy_validate,
};
@@ -379,16 +195,12 @@ static struct rtnl_link_ops dummy_link_ops __read_mostly = {
module_param(numdummies, int, 0);
MODULE_PARM_DESC(numdummies, "Number of dummy pseudo devices");
-module_param(num_vfs, int, 0);
-MODULE_PARM_DESC(num_vfs, "Number of dummy VFs per dummy device");
-
static int __init dummy_init_one(void)
{
struct net_device *dev_dummy;
int err;
- dev_dummy = alloc_netdev(sizeof(struct dummy_priv),
- "dummy%d", NET_NAME_ENUM, dummy_setup);
+ dev_dummy = alloc_netdev(0, "dummy%d", NET_NAME_ENUM, dummy_setup);
if (!dev_dummy)
return -ENOMEM;
@@ -407,21 +219,6 @@ static int __init dummy_init_module(void)
{
int i, err = 0;
- if (num_vfs) {
- err = bus_register(&dummy_bus);
- if (err < 0) {
- pr_err("registering dummy bus failed\n");
- return err;
- }
-
- err = device_register(&dummy_parent);
- if (err < 0) {
- pr_err("registering dummy parent device failed\n");
- bus_unregister(&dummy_bus);
- return err;
- }
- }
-
rtnl_lock();
err = __rtnl_link_register(&dummy_link_ops);
if (err < 0)
@@ -437,22 +234,12 @@ static int __init dummy_init_module(void)
out:
rtnl_unlock();
- if (err && num_vfs) {
- device_unregister(&dummy_parent);
- bus_unregister(&dummy_bus);
- }
-
return err;
}
static void __exit dummy_cleanup_module(void)
{
rtnl_link_unregister(&dummy_link_ops);
-
- if (num_vfs) {
- device_unregister(&dummy_parent);
- bus_unregister(&dummy_bus);
- }
}
module_init(dummy_init_module);
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 7/8] netdevsim: add SR-IOV functionality
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev
Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski, Phil Sutter, Sabrina Dubroca
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
dummy driver was extended with VF-related netdev APIs for testing
SR-IOV-related software. netdevsim did not exist back then.
Implement SR-IOV functionality in netdevsim. Notable difference
is that since netdevsim has no module parameters, we will actually
create a device with sriov_numvfs attribute for each netdev.
The zero MAC address is accepted as some HW use it to mean any
address is allowed. Link state is also now validated.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
CC: Phil Sutter <phil@nwl.cc>
CC: Sabrina Dubroca <sd@queasysnail.net>
drivers/net/netdevsim/netdev.c | 266 +++++++++++++++++++++++++++++++++++++-
drivers/net/netdevsim/netdevsim.h | 9 ++
2 files changed, 274 insertions(+), 1 deletion(-)
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 828c1ce49a8b..26c2adccff6f 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -25,6 +25,117 @@
#include "netdevsim.h"
+struct nsim_vf_config {
+ int link_state;
+ u16 min_tx_rate;
+ u16 max_tx_rate;
+ u16 vlan;
+ __be16 vlan_proto;
+ u16 qos;
+ u8 vf_mac[ETH_ALEN];
+ bool spoofchk_enabled;
+ bool trusted;
+ bool rss_query_enabled;
+};
+
+static int nsim_num_vf(struct device *dev)
+{
+ struct netdevsim *ns = to_nsim(dev);
+
+ return ns->num_vfs;
+}
+
+static struct bus_type nsim_bus = {
+ .name = DRV_NAME,
+ .num_vf = nsim_num_vf,
+};
+
+static void nsim_release_dev(struct device *dev)
+{
+}
+
+static int nsim_vfs_enable(struct netdevsim *ns, unsigned int num_vfs)
+{
+ ns->vfconfigs = kcalloc(num_vfs, sizeof(struct nsim_vf_config),
+ GFP_KERNEL);
+ if (!ns->vfconfigs)
+ return -ENOMEM;
+ ns->num_vfs = num_vfs;
+
+ return 0;
+}
+
+static void nsim_vfs_disable(struct netdevsim *ns)
+{
+ kfree(ns->vfconfigs);
+ ns->vfconfigs = NULL;
+ ns->num_vfs = 0;
+}
+
+static ssize_t
+nsim_numvfs_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct netdevsim *ns = to_nsim(dev);
+ unsigned int num_vfs;
+ int ret;
+
+ ret = kstrtouint(buf, 0, &num_vfs);
+ if (ret)
+ return ret;
+
+ rtnl_lock();
+ if (ns->num_vfs == num_vfs)
+ goto exit_good;
+ if (ns->num_vfs && num_vfs) {
+ ret = -EBUSY;
+ goto exit_unlock;
+ }
+
+ if (num_vfs) {
+ ret = nsim_vfs_enable(ns, num_vfs);
+ if (ret)
+ goto exit_unlock;
+ } else {
+ nsim_vfs_disable(ns);
+ }
+exit_good:
+ ret = count;
+exit_unlock:
+ rtnl_unlock();
+
+ return ret;
+}
+
+static ssize_t
+nsim_numvfs_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct netdevsim *ns = to_nsim(dev);
+
+ return sprintf(buf, "%u\n", ns->num_vfs);
+}
+
+static struct device_attribute nsim_numvfs_attr =
+ __ATTR(sriov_numvfs, 0664, nsim_numvfs_show, nsim_numvfs_store);
+
+static struct attribute *nsim_dev_attrs[] = {
+ &nsim_numvfs_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group nsim_dev_attr_group = {
+ .attrs = nsim_dev_attrs,
+};
+
+static const struct attribute_group *nsim_dev_attr_groups[] = {
+ &nsim_dev_attr_group,
+ NULL,
+};
+
+struct device_type nsim_dev_type = {
+ .groups = nsim_dev_attr_groups,
+};
+
static int nsim_init(struct net_device *dev)
{
struct netdevsim *ns = netdev_priv(dev);
@@ -37,8 +148,20 @@ static int nsim_init(struct net_device *dev)
if (err)
goto err_debugfs_destroy;
+ ns->dev.init_name = netdev_name(dev);
+ ns->dev.bus = &nsim_bus;
+ ns->dev.release = nsim_release_dev;
+ ns->dev.type = &nsim_dev_type;
+ err = device_register(&ns->dev);
+ if (err)
+ goto err_bpf_uninit;
+
+ SET_NETDEV_DEV(dev, &ns->dev);
+
return 0;
+err_bpf_uninit:
+ nsim_bpf_uninit(ns);
err_debugfs_destroy:
debugfs_remove_recursive(ns->ddir);
return err;
@@ -50,6 +173,14 @@ static void nsim_uninit(struct net_device *dev)
debugfs_remove_recursive(ns->ddir);
nsim_bpf_uninit(ns);
+ nsim_vfs_disable(ns);
+}
+
+static void nsim_free(struct net_device *dev)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ device_unregister(&ns->dev);
}
static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -122,6 +253,123 @@ nsim_setup_tc_block(struct net_device *dev, struct tc_block_offload *f)
}
}
+static int nsim_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ /* Only refuse multicast addresses, zero address can mean unset/any. */
+ if (vf >= ns->num_vfs || is_multicast_ether_addr(mac))
+ return -EINVAL;
+ memcpy(ns->vfconfigs[vf].vf_mac, mac, ETH_ALEN);
+
+ return 0;
+}
+
+static int nsim_set_vf_vlan(struct net_device *dev, int vf,
+ u16 vlan, u8 qos, __be16 vlan_proto)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (vf >= ns->num_vfs || vlan > 4095 || qos > 7)
+ return -EINVAL;
+
+ ns->vfconfigs[vf].vlan = vlan;
+ ns->vfconfigs[vf].qos = qos;
+ ns->vfconfigs[vf].vlan_proto = vlan_proto;
+
+ return 0;
+}
+
+static int nsim_set_vf_rate(struct net_device *dev, int vf, int min, int max)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (vf >= ns->num_vfs)
+ return -EINVAL;
+
+ ns->vfconfigs[vf].min_tx_rate = min;
+ ns->vfconfigs[vf].max_tx_rate = max;
+
+ return 0;
+}
+
+static int nsim_set_vf_spoofchk(struct net_device *dev, int vf, bool val)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (vf >= ns->num_vfs)
+ return -EINVAL;
+ ns->vfconfigs[vf].spoofchk_enabled = val;
+
+ return 0;
+}
+
+static int nsim_set_vf_rss_query_en(struct net_device *dev, int vf, bool val)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (vf >= ns->num_vfs)
+ return -EINVAL;
+ ns->vfconfigs[vf].rss_query_enabled = val;
+
+ return 0;
+}
+
+static int nsim_set_vf_trust(struct net_device *dev, int vf, bool val)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (vf >= ns->num_vfs)
+ return -EINVAL;
+ ns->vfconfigs[vf].trusted = val;
+
+ return 0;
+}
+
+static int
+nsim_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (vf >= ns->num_vfs)
+ return -EINVAL;
+
+ ivi->vf = vf;
+ ivi->linkstate = ns->vfconfigs[vf].link_state;
+ ivi->min_tx_rate = ns->vfconfigs[vf].min_tx_rate;
+ ivi->max_tx_rate = ns->vfconfigs[vf].max_tx_rate;
+ ivi->vlan = ns->vfconfigs[vf].vlan;
+ ivi->vlan_proto = ns->vfconfigs[vf].vlan_proto;
+ ivi->qos = ns->vfconfigs[vf].qos;
+ memcpy(&ivi->mac, ns->vfconfigs[vf].vf_mac, ETH_ALEN);
+ ivi->spoofchk = ns->vfconfigs[vf].spoofchk_enabled;
+ ivi->trusted = ns->vfconfigs[vf].trusted;
+ ivi->rss_query_en = ns->vfconfigs[vf].rss_query_enabled;
+
+ return 0;
+}
+
+static int nsim_set_vf_link_state(struct net_device *dev, int vf, int state)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (vf >= ns->num_vfs)
+ return -EINVAL;
+
+ switch (state) {
+ case IFLA_VF_LINK_STATE_AUTO:
+ case IFLA_VF_LINK_STATE_ENABLE:
+ case IFLA_VF_LINK_STATE_DISABLE:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ns->vfconfigs[vf].link_state = state;
+
+ return 0;
+}
+
static int
nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
{
@@ -153,6 +401,14 @@ static const struct net_device_ops nsim_netdev_ops = {
.ndo_validate_addr = eth_validate_addr,
.ndo_change_mtu = nsim_change_mtu,
.ndo_get_stats64 = nsim_get_stats64,
+ .ndo_set_vf_mac = nsim_set_vf_mac,
+ .ndo_set_vf_vlan = nsim_set_vf_vlan,
+ .ndo_set_vf_rate = nsim_set_vf_rate,
+ .ndo_set_vf_spoofchk = nsim_set_vf_spoofchk,
+ .ndo_set_vf_trust = nsim_set_vf_trust,
+ .ndo_get_vf_config = nsim_get_vf_config,
+ .ndo_set_vf_link_state = nsim_set_vf_link_state,
+ .ndo_set_vf_rss_query_en = nsim_set_vf_rss_query_en,
.ndo_setup_tc = nsim_setup_tc,
.ndo_set_features = nsim_set_features,
.ndo_bpf = nsim_bpf,
@@ -165,6 +421,7 @@ static void nsim_setup(struct net_device *dev)
dev->netdev_ops = &nsim_netdev_ops;
dev->needs_free_netdev = true;
+ dev->priv_destructor = nsim_free;
dev->tx_queue_len = 0;
dev->flags |= IFF_NOARP;
@@ -209,12 +466,18 @@ static int __init nsim_module_init(void)
if (IS_ERR(nsim_ddir))
return PTR_ERR(nsim_ddir);
- err = rtnl_link_register(&nsim_link_ops);
+ err = bus_register(&nsim_bus);
if (err)
goto err_debugfs_destroy;
+ err = rtnl_link_register(&nsim_link_ops);
+ if (err)
+ goto err_unreg_bus;
+
return 0;
+err_unreg_bus:
+ bus_unregister(&nsim_bus);
err_debugfs_destroy:
debugfs_remove_recursive(nsim_ddir);
return err;
@@ -223,6 +486,7 @@ static int __init nsim_module_init(void)
static void __exit nsim_module_exit(void)
{
rtnl_link_unregister(&nsim_link_ops);
+ bus_unregister(&nsim_bus);
debugfs_remove_recursive(nsim_ddir);
}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 8779e6a8f885..7bb1a0230940 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -13,6 +13,7 @@
* THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
*/
+#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/netdevice.h>
@@ -26,6 +27,9 @@
struct bpf_prog;
struct dentry;
+struct nsim_vf_config;
+
+#define to_nsim(ptr) container_of(ptr, struct netdevsim, dev)
struct netdevsim {
struct net_device *netdev;
@@ -34,8 +38,13 @@ struct netdevsim {
u64 tx_bytes;
struct u64_stats_sync syncp;
+ struct device dev;
+
struct dentry *ddir;
+ unsigned int num_vfs;
+ struct nsim_vf_config *vfconfigs;
+
struct bpf_prog *bpf_offloaded;
u32 bpf_offloaded_id;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 6/8] selftests/bpf: add offload test based on netdevsim
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev; +Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
Add a test of BPF offload control path interfaces based on
just-added netdevsim driver. Perform various checks of both
the stack and the expected driver behaviour.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/testing/selftests/bpf/Makefile | 5 +-
tools/testing/selftests/bpf/sample_ret0.c | 7 +
tools/testing/selftests/bpf/test_offload.py | 681 ++++++++++++++++++++++++++++
3 files changed, 691 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/sample_ret0.c
create mode 100755 tools/testing/selftests/bpf/test_offload.py
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 333a48655ee0..2c9d8c63c6fa 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -17,9 +17,10 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o \
- sockmap_verdict_prog.o dev_cgroup.o
+ sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o
-TEST_PROGS := test_kmod.sh test_xdp_redirect.sh test_xdp_meta.sh
+TEST_PROGS := test_kmod.sh test_xdp_redirect.sh test_xdp_meta.sh \
+ test_offload.py
include ../lib.mk
diff --git a/tools/testing/selftests/bpf/sample_ret0.c b/tools/testing/selftests/bpf/sample_ret0.c
new file mode 100644
index 000000000000..fec99750d6ea
--- /dev/null
+++ b/tools/testing/selftests/bpf/sample_ret0.c
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
+
+/* Sample program which should always load for testing control paths. */
+int func()
+{
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
new file mode 100755
index 000000000000..3914f7a4585a
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -0,0 +1,681 @@
+#!/usr/bin/python3
+
+# Copyright (C) 2017 Netronome Systems, Inc.
+#
+# This software is licensed under the GNU General License Version 2,
+# June 1991 as shown in the file COPYING in the top-level directory of this
+# source tree.
+#
+# THE COPYRIGHT HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS"
+# WITHOUT WARRANTY OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING,
+# BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+# FOR A PARTICULAR PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE
+# OF THE PROGRAM IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME
+# THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
+
+from datetime import datetime
+import argparse
+import json
+import os
+import pprint
+import subprocess
+import time
+
+logfile = None
+log_level = 1
+bpf_test_dir = os.path.dirname(os.path.realpath(__file__))
+pp = pprint.PrettyPrinter()
+devs = [] # devices we created for clean up
+files = [] # files to be removed
+
+def log_get_sec(level=0):
+ return "*" * (log_level + level)
+
+def log_level_inc(add=1):
+ global log_level
+ log_level += add
+
+def log_level_dec(sub=1):
+ global log_level
+ log_level -= sub
+
+def log_level_set(level):
+ global log_level
+ log_level = level
+
+def log(header, data, level=None):
+ """
+ Output to an optional log.
+ """
+ if logfile is None:
+ return
+ if level is not None:
+ log_level_set(level)
+
+ if not isinstance(data, str):
+ data = pp.pformat(data)
+
+ if len(header):
+ logfile.write("\n" + log_get_sec() + " ")
+ logfile.write(header)
+ if len(header) and len(data.strip()):
+ logfile.write("\n")
+ logfile.write(data)
+
+def skip(cond, msg):
+ if not cond:
+ return
+ print("SKIP: " + msg)
+ log("SKIP: " + msg, "", level=1)
+ os.sys.exit(0)
+
+def fail(cond, msg):
+ if not cond:
+ return
+ print("FAIL: " + msg)
+ log("FAIL: " + msg, "", level=1)
+ os.sys.exit(1)
+
+def start_test(msg):
+ log(msg, "", level=1)
+ log_level_inc()
+ print(msg)
+
+def cmd(cmd, shell=True, include_stderr=False, background=False, fail=True):
+ """
+ Run a command in subprocess and return tuple of (retval, stdout);
+ optionally return stderr as well as third value.
+ """
+ proc = subprocess.Popen(cmd, shell=shell, stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+ if background:
+ msg = "%s START: %s" % (log_get_sec(1),
+ datetime.now().strftime("%H:%M:%S.%f"))
+ log("BKG " + proc.args, msg)
+ return proc
+
+ return cmd_result(proc, include_stderr=include_stderr, fail=fail)
+
+def cmd_result(proc, include_stderr=False, fail=False):
+ stdout, stderr = proc.communicate()
+ stdout = stdout.decode("utf-8")
+ stderr = stderr.decode("utf-8")
+ proc.stdout.close()
+ proc.stderr.close()
+
+ stderr = "\n" + stderr
+ if stderr[-1] == "\n":
+ stderr = stderr[:-1]
+
+ sec = log_get_sec(1)
+ log("CMD " + proc.args,
+ "RETCODE: %d\n%s STDOUT:\n%s%s STDERR:%s\n%s END: %s" %
+ (proc.returncode, sec, stdout, sec, stderr,
+ sec, datetime.now().strftime("%H:%M:%S.%f")))
+
+ if proc.returncode != 0 and fail:
+ if len(stderr) > 0 and stderr[-1] == "\n":
+ stderr = stderr[:-1]
+ raise Exception("Command failed: %s\n%s" % (proc.args, stderr))
+
+ if include_stderr:
+ return proc.returncode, stdout, stderr
+ else:
+ return proc.returncode, stdout
+
+def rm(f):
+ cmd("rm -f %s" % (f))
+ if f in files:
+ files.remove(f)
+
+def tool(name, args, flags, JSON=True, fail=True):
+ params = ""
+ if JSON:
+ params += "%s " % (flags["json"])
+
+ ret, out = cmd(name + " " + params + args, fail=fail)
+ if JSON and len(out.strip()) != 0:
+ return ret, json.loads(out)
+ else:
+ return ret, out
+
+def bpftool(args, JSON=True, fail=True):
+ return tool("bpftool", args, {"json":"-p"}, JSON=JSON, fail=fail)
+
+def bpftool_prog_list(expected=None):
+ _, progs = bpftool("prog show", JSON=True, fail=True)
+ if expected is not None:
+ if len(progs) != expected:
+ fail(True, "%d BPF programs loaded, expected %d" %
+ (len(progs), expected))
+ return progs
+
+def bpftool_prog_list_wait(expected=0, n_retry=20):
+ for i in range(n_retry):
+ nprogs = len(bpftool_prog_list())
+ if nprogs == expected:
+ return
+ time.sleep(0.05)
+ raise Exception("Time out waiting for program counts to stabilize want %d, have %d" % (expected, nprogs))
+
+def ip(args, force=False, JSON=True, fail=True):
+ if force:
+ args = "-force " + args
+ return tool("ip", args, {"json":"-j"}, JSON=JSON, fail=fail)
+
+def tc(args, JSON=True, fail=True):
+ return tool("tc", args, {"json":"-p"}, JSON=JSON, fail=fail)
+
+def ethtool(dev, opt, args, fail=True):
+ return cmd("ethtool %s %s %s" % (opt, dev["ifname"], args), fail=fail)
+
+def bpf_obj(name, sec=".text", path=bpf_test_dir,):
+ return "obj %s sec %s" % (os.path.join(path, name), sec)
+
+def bpf_pinned(name):
+ return "pinned %s" % (name)
+
+def bpf_bytecode(bytecode):
+ return "bytecode \"%s\"" % (bytecode)
+
+class DebugfsDir:
+ """
+ Class for accessing DebugFS directories as a dictionary.
+ """
+
+ def __init__(self, path):
+ self.path = path
+ self._dict = self._debugfs_dir_read(path)
+
+ def __len__(self):
+ return len(self._dict.keys())
+
+ def __getitem__(self, key):
+ if type(key) is int:
+ key = list(self._dict.keys())[key]
+ return self._dict[key]
+
+ def __setitem__(self, key, value):
+ log("DebugFS set %s = %s" % (key, value), "")
+ log_level_inc()
+
+ cmd("echo '%s' > %s/%s" % (value, self.path, key))
+ log_level_dec()
+
+ _, out = cmd('cat %s/%s' % (self.path, key))
+ self._dict[key] = out.strip()
+
+ def _debugfs_dir_read(self, path):
+ dfs = {}
+
+ log("DebugFS state for %s" % (path), "")
+ log_level_inc(add=2)
+
+ _, out = cmd('ls ' + path)
+ for f in out.split():
+ p = os.path.join(path, f)
+ if os.path.isfile(p):
+ _, out = cmd('cat %s/%s' % (path, f))
+ dfs[f] = out.strip()
+ elif os.path.isdir(p):
+ dfs[f] = DebugfsDir(p)
+ else:
+ raise Exception("%s is neither file nor directory" % (p))
+
+ log_level_dec()
+ log("DebugFS state", dfs)
+ log_level_dec()
+
+ return dfs
+
+class NetdevSim:
+ """
+ Class for netdevsim netdevice and its attributes.
+ """
+
+ def __init__(self):
+ self.dev = self._netdevsim_create()
+ devs.append(self)
+
+ self.dfs_dir = '/sys/kernel/debug/netdevsim/%s' % (self.dev['ifname'])
+ self.dfs_refresh()
+
+ def __getitem__(self, key):
+ return self.dev[key]
+
+ def _netdevsim_create(self):
+ _, old = ip("link show")
+ ip("link add sim%d type netdevsim")
+ _, new = ip("link show")
+
+ for dev in new:
+ f = filter(lambda x: x["ifname"] == dev["ifname"], old)
+ if len(list(f)) == 0:
+ return dev
+
+ raise Exception("failed to create netdevsim device")
+
+ def remove(self):
+ devs.remove(self)
+ ip("link del dev %s" % (self.dev["ifname"]))
+
+ def dfs_refresh(self):
+ self.dfs = DebugfsDir(self.dfs_dir)
+ return self.dfs
+
+ def dfs_num_bound_progs(self):
+ path = os.path.join(self.dfs_dir, "bpf_bound_progs")
+ _, progs = cmd('ls %s' % (path))
+ return len(progs.split())
+
+ def dfs_get_bound_progs(self, expected):
+ progs = DebugfsDir(os.path.join(self.dfs_dir, "bpf_bound_progs"))
+ if expected is not None:
+ if len(progs) != expected:
+ fail(True, "%d BPF programs bound, expected %d" %
+ (len(progs), expected))
+ return progs
+
+ def wait_for_flush(self, bound=0, total=0, n_retry=20):
+ for i in range(n_retry):
+ nbound = self.dfs_num_bound_progs()
+ nprogs = len(bpftool_prog_list())
+ if nbound == bound and nprogs == total:
+ return
+ time.sleep(0.05)
+ raise Exception("Time out waiting for program counts to stabilize want %d/%d, have %d bound, %d loaded" % (bound, total, nbound, nprogs))
+
+ def set_mtu(self, mtu, fail=True):
+ return ip("link set dev %s mtu %d" % (self.dev["ifname"], mtu),
+ fail=fail)
+
+ def set_xdp(self, bpf, mode, force=False, fail=True):
+ return ip("link set dev %s xdp%s %s" % (self.dev["ifname"], mode, bpf),
+ force=force, fail=fail)
+
+ def unset_xdp(self, mode, force=False, fail=True):
+ return ip("link set dev %s xdp%s off" % (self.dev["ifname"], mode),
+ force=force, fail=fail)
+
+ def ip_link_show(self, xdp):
+ _, link = ip("link show dev %s" % (self['ifname']))
+ if len(link) > 1:
+ raise Exception("Multiple objects on ip link show")
+ if len(link) < 1:
+ return {}
+ fail(xdp != "xdp" in link,
+ "XDP program not reporting in iplink (reported %s, expected %s)" %
+ ("xdp" in link, xdp))
+ return link[0]
+
+ def tc_add_ingress(self):
+ tc("qdisc add dev %s ingress" % (self['ifname']))
+
+ def tc_del_ingress(self):
+ tc("qdisc del dev %s ingress" % (self['ifname']))
+
+ def tc_flush_filters(self, bound=0, total=0):
+ self.tc_del_ingress()
+ self.tc_add_ingress()
+ self.wait_for_flush(bound=bound, total=total)
+
+ def tc_show_ingress(self, expected=None):
+ # No JSON support, oh well...
+ flags = ["skip_sw", "skip_hw", "in_hw"]
+ named = ["protocol", "pref", "chain", "handle", "id", "tag"]
+
+ args = "-s filter show dev %s ingress" % (self['ifname'])
+ _, out = tc(args, JSON=False)
+
+ filters = []
+ lines = out.split('\n')
+ for line in lines:
+ words = line.split()
+ if "handle" not in words:
+ continue
+ fltr = {}
+ for flag in flags:
+ fltr[flag] = flag in words
+ for name in named:
+ try:
+ idx = words.index(name)
+ fltr[name] = words[idx + 1]
+ except ValueError:
+ pass
+ filters.append(fltr)
+
+ if expected is not None:
+ fail(len(filters) != expected,
+ "%d ingress filters loaded, expected %d" %
+ (len(filters), expected))
+ return filters
+
+ def cls_bpf_add_filter(self, bpf, da=False, skip_sw=False, skip_hw=False,
+ fail=True):
+ params = ""
+ if da:
+ params += " da"
+ if skip_sw:
+ params += " skip_sw"
+ if skip_hw:
+ params += " skip_hw"
+ return tc("filter add dev %s ingress bpf %s %s" %
+ (self['ifname'], bpf, params), fail=fail)
+
+ def set_ethtool_tc_offloads(self, enable, fail=True):
+ args = "hw-tc-offload %s" % ("on" if enable else "off")
+ return ethtool(self, "-K", args, fail=fail)
+
+################################################################################
+def clean_up():
+ for dev in devs:
+ dev.remove()
+ for f in files:
+ cmd("rm -f %s" % (f))
+
+def pin_prog(file_name, idx=0):
+ progs = bpftool_prog_list(expected=(idx + 1))
+ prog = progs[idx]
+ bpftool("prog pin id %d %s" % (prog["id"], file_name))
+ files.append(file_name)
+
+ return file_name, bpf_pinned(file_name)
+
+# Parse command line
+parser = argparse.ArgumentParser()
+parser.add_argument("--log", help="output verbose log to given file")
+args = parser.parse_args()
+if args.log:
+ logfile = open(args.log, 'w+')
+ logfile.write("# -*-Org-*-")
+
+log("Prepare...", "", level=1)
+log_level_inc()
+
+# Check permissions
+skip(os.getuid() != 0, "test must be run as root")
+
+# Check tools
+ret, progs = bpftool("prog", fail=False)
+skip(ret != 0, "bpftool not installed")
+# Check no BPF programs are loaded
+skip(len(progs) != 0, "BPF programs already loaded on the system")
+
+# Check netdevsim
+ret, out = cmd("modprobe netdevsim", fail=False)
+skip(ret != 0, "netdevsim module could not be loaded")
+
+# Check debugfs
+_, out = cmd("mount")
+if out.find("/sys/kernel/debug type debugfs") == -1:
+ cmd("mount -t debugfs none /sys/kernel/debug")
+
+# Check samples are compiled
+samples = ["sample_ret0.o"]
+for s in samples:
+ ret, out = cmd("ls %s/%s" % (bpf_test_dir, s), fail=False)
+ skip(ret != 0, "sample %s/%s not found, please compile it" %
+ (bpf_test_dir, s))
+
+try:
+ obj = bpf_obj("sample_ret0.o")
+ bytecode = bpf_bytecode("1,6 0 0 4294967295,")
+
+ start_test("Test destruction of generic XDP...")
+ sim = NetdevSim()
+ sim.set_xdp(obj, "generic")
+ sim.remove()
+ bpftool_prog_list_wait(expected=0)
+
+ sim = NetdevSim()
+ sim.tc_add_ingress()
+
+ start_test("Test TC non-offloaded...")
+ ret, _ = sim.cls_bpf_add_filter(obj, skip_hw=True, fail=False)
+ fail(ret != 0, "Software TC filter did not load")
+
+ start_test("Test TC non-offloaded isn't getting bound...")
+ ret, _ = sim.cls_bpf_add_filter(obj, fail=False)
+ fail(ret != 0, "Software TC filter did not load")
+ sim.dfs_get_bound_progs(expected=0)
+
+ sim.tc_flush_filters()
+
+ start_test("Test TC offloads are off by default...")
+ ret, _ = sim.cls_bpf_add_filter(obj, skip_sw=True, fail=False)
+ fail(ret == 0, "TC filter loaded without enabling TC offloads")
+ sim.wait_for_flush()
+
+ sim.set_ethtool_tc_offloads(True)
+ sim.dfs["bpf_tc_non_bound_accept"] = "Y"
+
+ start_test("Test TC offload by default...")
+ ret, _ = sim.cls_bpf_add_filter(obj, fail=False)
+ fail(ret != 0, "Software TC filter did not load")
+ sim.dfs_get_bound_progs(expected=0)
+ ingress = sim.tc_show_ingress(expected=1)
+ fltr = ingress[0]
+ fail(not fltr["in_hw"], "Filter not offloaded by default")
+
+ sim.tc_flush_filters()
+
+ start_test("Test TC cBPF bytcode tries offload by default...")
+ ret, _ = sim.cls_bpf_add_filter(bytecode, fail=False)
+ fail(ret != 0, "Software TC filter did not load")
+ sim.dfs_get_bound_progs(expected=0)
+ ingress = sim.tc_show_ingress(expected=1)
+ fltr = ingress[0]
+ fail(not fltr["in_hw"], "Bytecode not offloaded by default")
+
+ sim.tc_flush_filters()
+ sim.dfs["bpf_tc_non_bound_accept"] = "N"
+
+ start_test("Test TC cBPF unbound bytecode doesn't offload...")
+ ret, _ = sim.cls_bpf_add_filter(bytecode, skip_sw=True, fail=False)
+ fail(ret == 0, "TC bytecode loaded for offload")
+ sim.wait_for_flush()
+
+ start_test("Test TC offloads work...")
+ ret, _ = sim.cls_bpf_add_filter(obj, skip_sw=True, fail=False)
+ fail(ret != 0, "TC filter did not load with TC offloads enabled")
+
+ start_test("Test TC offload basics...")
+ dfs = sim.dfs_get_bound_progs(expected=1)
+ progs = bpftool_prog_list(expected=1)
+ ingress = sim.tc_show_ingress(expected=1)
+
+ dprog = dfs[0]
+ prog = progs[0]
+ fltr = ingress[0]
+ fail(fltr["skip_hw"], "TC does reports 'skip_hw' on offloaded filter")
+ fail(not fltr["in_hw"], "TC does not report 'in_hw' for offloaded filter")
+ fail(not fltr["skip_sw"], "TC does not report 'skip_sw' back")
+
+ start_test("Test TC offload is device-bound...")
+ fail(str(prog["id"]) != fltr["id"], "Program IDs don't match")
+ fail(prog["tag"] != fltr["tag"], "Program tags don't match")
+ fail(fltr["id"] != dprog["id"], "Program IDs don't match")
+ fail(dprog["state"] != "xlated", "Offloaded program state not translated")
+ fail(dprog["loaded"] != "Y", "Offloaded program is not loaded")
+
+ start_test("Test disabling TC offloads is rejected while filters installed...")
+ ret, _ = sim.set_ethtool_tc_offloads(False, fail=False)
+ fail(ret == 0, "Driver should refuse to disable TC offloads with filters installed...")
+
+ start_test("Test qdisc removal frees things...")
+ sim.tc_flush_filters()
+ sim.tc_show_ingress(expected=0)
+
+ start_test("Test disabling TC offloads is OK without filters...")
+ ret, _ = sim.set_ethtool_tc_offloads(False, fail=False)
+ fail(ret != 0,
+ "Driver refused to disable TC offloads without filters installed...")
+
+ sim.set_ethtool_tc_offloads(True)
+
+ start_test("Test destroying device gets rid of TC filters...")
+ sim.cls_bpf_add_filter(obj, skip_sw=True)
+ sim.remove()
+ bpftool_prog_list_wait(expected=0)
+
+ sim = NetdevSim()
+ sim.set_ethtool_tc_offloads(True)
+
+ start_test("Test destroying device gets rid of XDP...")
+ sim.set_xdp(obj, "offload")
+ sim.remove()
+ bpftool_prog_list_wait(expected=0)
+
+ sim = NetdevSim()
+ sim.set_ethtool_tc_offloads(True)
+
+ start_test("Test XDP prog reporting...")
+ sim.set_xdp(obj, "drv")
+ ipl = sim.ip_link_show(xdp=True)
+ progs = bpftool_prog_list(expected=1)
+ fail(ipl["xdp"]["prog"]["id"] != progs[0]["id"],
+ "Loaded program has wrong ID")
+
+ start_test("Test XDP prog replace without force...")
+ ret, _ = sim.set_xdp(obj, "drv", fail=False)
+ fail(ret == 0, "Replaced XDP program without -force")
+ sim.wait_for_flush(total=1)
+
+ start_test("Test XDP prog replace with force...")
+ ret, _ = sim.set_xdp(obj, "drv", force=True, fail=False)
+ fail(ret != 0, "Could not replace XDP program with -force")
+ bpftool_prog_list_wait(expected=1)
+ ipl = sim.ip_link_show(xdp=True)
+ progs = bpftool_prog_list(expected=1)
+ fail(ipl["xdp"]["prog"]["id"] != progs[0]["id"],
+ "Loaded program has wrong ID")
+
+ start_test("Test XDP prog replace with bad flags...")
+ ret, _ = sim.set_xdp(obj, "offload", force=True, fail=False)
+ fail(ret == 0, "Replaced XDP program with a program in different mode")
+ ret, _ = sim.set_xdp(obj, "", force=True, fail=False)
+ fail(ret == 0, "Replaced XDP program with a program in different mode")
+
+ start_test("Test XDP prog remove with bad flags...")
+ ret, _ = sim.unset_xdp("offload", force=True, fail=False)
+ fail(ret == 0, "Removed program with a bad mode mode")
+ ret, _ = sim.unset_xdp("", force=True, fail=False)
+ fail(ret == 0, "Removed program with a bad mode mode")
+
+ start_test("Test MTU restrictions...")
+ ret, _ = sim.set_mtu(9000, fail=False)
+ fail(ret == 0,
+ "Driver should refuse to increase MTU to 9000 with XDP loaded...")
+ sim.unset_xdp("drv")
+ bpftool_prog_list_wait(expected=0)
+ sim.set_mtu(9000)
+ ret, _ = sim.set_xdp(obj, "drv", fail=False)
+ fail(ret == 0, "Driver should refuse to load program with MTU of 9000...")
+ sim.set_mtu(1500)
+
+ sim.wait_for_flush()
+ start_test("Test XDP offload...")
+ sim.set_xdp(obj, "offload")
+ ipl = sim.ip_link_show(xdp=True)
+ link_xdp = ipl["xdp"]["prog"]
+ progs = bpftool_prog_list(expected=1)
+ prog = progs[0]
+ fail(link_xdp["id"] != prog["id"], "Loaded program has wrong ID")
+
+ start_test("Test XDP offload is device bound...")
+ dfs = sim.dfs_get_bound_progs(expected=1)
+ dprog = dfs[0]
+
+ fail(prog["id"] != link_xdp["id"], "Program IDs don't match")
+ fail(prog["tag"] != link_xdp["tag"], "Program tags don't match")
+ fail(str(link_xdp["id"]) != dprog["id"], "Program IDs don't match")
+ fail(dprog["state"] != "xlated", "Offloaded program state not translated")
+ fail(dprog["loaded"] != "Y", "Offloaded program is not loaded")
+
+ start_test("Test removing XDP program many times...")
+ sim.unset_xdp("offload")
+ sim.unset_xdp("offload")
+ sim.unset_xdp("drv")
+ sim.unset_xdp("drv")
+ sim.unset_xdp("")
+ sim.unset_xdp("")
+ bpftool_prog_list_wait(expected=0)
+
+ start_test("Test attempt to use a program for a wrong device...")
+ sim2 = NetdevSim()
+ sim2.set_xdp(obj, "offload")
+ pin_file, pinned = pin_prog("/sys/fs/bpf/tmp")
+
+ ret, _ = sim.set_xdp(pinned, "offload", fail=False)
+ fail(ret == 0, "Pinned program loaded for a different device accepted")
+ sim2.remove()
+ ret, _ = sim.set_xdp(pinned, "offload", fail=False)
+ fail(ret == 0, "Pinned program loaded for a removed device accepted")
+ rm(pin_file)
+ bpftool_prog_list_wait(expected=0)
+
+ start_test("Test mixing of TC and XDP...")
+ sim.tc_add_ingress()
+ sim.set_xdp(obj, "offload")
+ ret, _ = sim.cls_bpf_add_filter(obj, skip_sw=True, fail=False)
+ fail(ret == 0, "Loading TC when XDP active should fail")
+ sim.unset_xdp("offload")
+ sim.wait_for_flush()
+
+ sim.cls_bpf_add_filter(obj, skip_sw=True)
+ ret, _ = sim.set_xdp(obj, "offload", fail=False)
+ fail(ret == 0, "Loading XDP when TC active should fail")
+
+ start_test("Test binding TC from pinned...")
+ pin_file, pinned = pin_prog("/sys/fs/bpf/tmp")
+ sim.tc_flush_filters(bound=1, total=1)
+ sim.cls_bpf_add_filter(pinned, da=True, skip_sw=True)
+ sim.tc_flush_filters(bound=1, total=1)
+
+ start_test("Test binding XDP from pinned...")
+ sim.set_xdp(obj, "offload")
+ pin_file, pinned = pin_prog("/sys/fs/bpf/tmp2", idx=1)
+
+ sim.set_xdp(pinned, "offload", force=True)
+ sim.unset_xdp("offload")
+ sim.set_xdp(pinned, "offload", force=True)
+ sim.unset_xdp("offload")
+
+ start_test("Test offload of wrong type fails...")
+ ret, _ = sim.cls_bpf_add_filter(pinned, da=True, skip_sw=True, fail=False)
+ fail(ret == 0, "Managed to attach XDP program to TC")
+
+ start_test("Test asking for TC offload of two filters...")
+ sim.cls_bpf_add_filter(obj, da=True, skip_sw=True)
+ sim.cls_bpf_add_filter(obj, da=True, skip_sw=True)
+ # The above will trigger a splat until TC cls_bpf drivers are fixed
+
+ sim.tc_flush_filters(bound=2, total=2)
+
+ start_test("Test if netdev removal waits for translation...")
+ delay_msec = 500
+ sim.dfs["bpf_bind_verifier_delay"] = delay_msec
+ start = time.time()
+ cmd_line = "tc filter add dev %s ingress bpf %s da skip_sw" % \
+ (sim['ifname'], obj)
+ tc_proc = cmd(cmd_line, background=True, fail=False)
+ # Wait for the verifier to start
+ while sim.dfs_num_bound_progs() <= 2:
+ pass
+ sim.remove()
+ end = time.time()
+ ret, _ = cmd_result(tc_proc, fail=False)
+ time_diff = end - start
+ log("Time", "start:\t%s\nend:\t%s\ndiff:\t%s" % (start, end, time_diff))
+
+ fail(ret == 0, "Managed to load TC filter on a unregistering device")
+ delay_sec = delay_msec * 0.001
+ fail(time_diff < delay_sec, "Removal process took %s, expected %s" %
+ (time_diff, delay_sec))
+
+ print("%s: OK" % (os.path.basename(__file__)))
+
+finally:
+ log("Clean up...", "", level=1)
+ log_level_inc()
+ clean_up()
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 5/8] netdevsim: add bpf offload support
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev; +Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
Add support for loading programs for netdevsim devices and
expose the related information via DebugFS. Both offload
of XDP and cls_bpf programs is supported.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/netdevsim/Makefile | 1 +
drivers/net/netdevsim/bpf.c | 373 ++++++++++++++++++++++++++++++++++++++
drivers/net/netdevsim/netdev.c | 116 +++++++++++-
drivers/net/netdevsim/netdevsim.h | 40 ++++
4 files changed, 529 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/netdevsim/bpf.c
diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index 07867bfe873b..074ddebbc41d 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_NETDEVSIM) += netdevsim.o
netdevsim-objs := \
netdev.o \
+ bpf.o \
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
new file mode 100644
index 000000000000..8e4398a50903
--- /dev/null
+++ b/drivers/net/netdevsim/bpf.c
@@ -0,0 +1,373 @@
+/*
+ * Copyright (C) 2017 Netronome Systems, Inc.
+ *
+ * This software is licensed under the GNU General License Version 2,
+ * June 1991 as shown in the file COPYING in the top-level directory of this
+ * source tree.
+ *
+ * THE COPYRIGHT HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS"
+ * WITHOUT WARRANTY OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE
+ * OF THE PROGRAM IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME
+ * THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
+ */
+
+#include <linux/bpf.h>
+#include <linux/bpf_verifier.h>
+#include <linux/debugfs.h>
+#include <linux/kernel.h>
+#include <linux/rtnetlink.h>
+#include <net/pkt_cls.h>
+
+#include "netdevsim.h"
+
+struct nsim_bpf_bound_prog {
+ struct netdevsim *ns;
+ struct bpf_prog *prog;
+ struct dentry *ddir;
+ const char *state;
+ bool is_loaded;
+ struct list_head l;
+};
+
+static int nsim_debugfs_bpf_string_read(struct seq_file *file, void *data)
+{
+ const char **str = file->private;
+
+ if (*str)
+ seq_printf(file, "%s\n", *str);
+
+ return 0;
+}
+
+static int nsim_debugfs_bpf_string_open(struct inode *inode, struct file *f)
+{
+ return single_open(f, nsim_debugfs_bpf_string_read, inode->i_private);
+}
+
+static const struct file_operations nsim_bpf_string_fops = {
+ .owner = THIS_MODULE,
+ .open = nsim_debugfs_bpf_string_open,
+ .release = single_release,
+ .read = seq_read,
+ .llseek = seq_lseek
+};
+
+static int
+nsim_bpf_verify_insn(struct bpf_verifier_env *env, int insn_idx, int prev_insn)
+{
+ struct nsim_bpf_bound_prog *state;
+
+ state = env->prog->aux->offload->dev_priv;
+ if (state->ns->bpf_bind_verifier_delay && !insn_idx)
+ msleep(state->ns->bpf_bind_verifier_delay);
+
+ return 0;
+}
+
+static const struct bpf_ext_analyzer_ops nsim_bpf_analyzer_ops = {
+ .insn_hook = nsim_bpf_verify_insn,
+};
+
+static bool nsim_xdp_offload_active(struct netdevsim *ns)
+{
+ return ns->xdp_prog_mode == XDP_ATTACHED_HW;
+}
+
+static void nsim_prog_set_loaded(struct bpf_prog *prog, bool loaded)
+{
+ struct nsim_bpf_bound_prog *state;
+
+ if (!prog || !prog->aux->offload)
+ return;
+
+ state = prog->aux->offload->dev_priv;
+ state->is_loaded = loaded;
+}
+
+static int
+nsim_bpf_offload(struct netdevsim *ns, struct bpf_prog *prog, bool oldprog)
+{
+ nsim_prog_set_loaded(ns->bpf_offloaded, false);
+
+ WARN(!!ns->bpf_offloaded != oldprog,
+ "bad offload state, expected offload %sto be active",
+ oldprog ? "" : "not ");
+ ns->bpf_offloaded = prog;
+ ns->bpf_offloaded_id = prog ? prog->aux->id : 0;
+ nsim_prog_set_loaded(prog, true);
+
+ return 0;
+}
+
+int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
+ void *type_data, void *cb_priv)
+{
+ struct tc_cls_bpf_offload *cls_bpf = type_data;
+ struct bpf_prog *prog = cls_bpf->prog;
+ struct netdevsim *ns = cb_priv;
+ bool skip_sw;
+
+ if (type != TC_SETUP_CLSBPF ||
+ !tc_can_offload(ns->netdev) ||
+ cls_bpf->common.protocol != htons(ETH_P_ALL) ||
+ cls_bpf->common.chain_index)
+ return -EOPNOTSUPP;
+
+ skip_sw = cls_bpf->gen_flags & TCA_CLS_FLAGS_SKIP_SW;
+
+ if (nsim_xdp_offload_active(ns))
+ return -EBUSY;
+
+ if (!ns->bpf_tc_accept)
+ return -EOPNOTSUPP;
+ /* Note: progs without skip_sw will probably not be dev bound */
+ if (prog && !prog->aux->offload && !ns->bpf_tc_non_bound_accept)
+ return -EOPNOTSUPP;
+
+ switch (cls_bpf->command) {
+ case TC_CLSBPF_REPLACE:
+ return nsim_bpf_offload(ns, prog, true);
+ case TC_CLSBPF_ADD:
+ return nsim_bpf_offload(ns, prog, false);
+ case TC_CLSBPF_DESTROY:
+ return nsim_bpf_offload(ns, NULL, true);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+int nsim_bpf_disable_tc(struct netdevsim *ns)
+{
+ if (ns->bpf_offloaded && !nsim_xdp_offload_active(ns))
+ return -EBUSY;
+ return 0;
+}
+
+static int nsim_xdp_offload_prog(struct netdevsim *ns, struct netdev_bpf *bpf)
+{
+ if (!nsim_xdp_offload_active(ns) && !bpf->prog)
+ return 0;
+ if (!nsim_xdp_offload_active(ns) && bpf->prog && ns->bpf_offloaded) {
+ NSIM_EA(bpf->extack, "TC program is already loaded");
+ return -EBUSY;
+ }
+
+ return nsim_bpf_offload(ns, bpf->prog, nsim_xdp_offload_active(ns));
+}
+
+static int nsim_xdp_set_prog(struct netdevsim *ns, struct netdev_bpf *bpf)
+{
+ int err;
+
+ if (ns->xdp_prog && (bpf->flags ^ ns->xdp_flags) & XDP_FLAGS_MODES) {
+ NSIM_EA(bpf->extack, "program loaded with different flags");
+ return -EBUSY;
+ }
+
+ if (bpf->command == XDP_SETUP_PROG && !ns->bpf_xdpdrv_accept) {
+ NSIM_EA(bpf->extack, "driver XDP disabled in DebugFS");
+ return -EOPNOTSUPP;
+ }
+ if (bpf->command == XDP_SETUP_PROG_HW && !ns->bpf_xdpoffload_accept) {
+ NSIM_EA(bpf->extack, "XDP offload disabled in DebugFS");
+ return -EOPNOTSUPP;
+ }
+
+ if (bpf->command == XDP_SETUP_PROG_HW) {
+ err = nsim_xdp_offload_prog(ns, bpf);
+ if (err)
+ return err;
+ }
+
+ if (ns->xdp_prog)
+ bpf_prog_put(ns->xdp_prog);
+
+ ns->xdp_prog = bpf->prog;
+ ns->xdp_flags = bpf->flags;
+
+ if (!bpf->prog)
+ ns->xdp_prog_mode = XDP_ATTACHED_NONE;
+ else if (bpf->command == XDP_SETUP_PROG)
+ ns->xdp_prog_mode = XDP_ATTACHED_DRV;
+ else
+ ns->xdp_prog_mode = XDP_ATTACHED_HW;
+
+ return 0;
+}
+
+int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
+{
+ struct nsim_bpf_bound_prog *state;
+ char name[16];
+ int err;
+
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ state->ns = ns;
+ state->prog = prog;
+ state->state = "verify";
+
+ /* Program id is not populated yet when we create the state. */
+ sprintf(name, "%u", ns->prog_id_gen++);
+ state->ddir = debugfs_create_dir(name, ns->ddir_bpf_bound_progs);
+ if (IS_ERR(state->ddir)) {
+ err = PTR_ERR(state->ddir);
+ kfree(state);
+ return err;
+ }
+
+ debugfs_create_u32("id", 0400, state->ddir, &prog->aux->id);
+ debugfs_create_file("state", 0400, state->ddir,
+ &state->state, &nsim_bpf_string_fops);
+ debugfs_create_bool("loaded", 0400, state->ddir, &state->is_loaded);
+
+ list_add_tail(&state->l, &ns->bpf_bound_progs);
+
+ prog->aux->offload->dev_priv = state;
+
+ return 0;
+}
+
+void nsim_bpf_destroy_prog(struct bpf_prog *prog)
+{
+ struct nsim_bpf_bound_prog *state;
+
+ state = prog->aux->offload->dev_priv;
+ WARN(state->is_loaded,
+ "offload state destroyed while program still bound");
+ debugfs_remove_recursive(state->ddir);
+ list_del(&state->l);
+ kfree(state);
+}
+
+static int nsim_setup_prog_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
+{
+ if (bpf->prog && bpf->prog->aux->offload) {
+ NSIM_EA(bpf->extack, "attempt to load offloaded prog to drv");
+ return -EINVAL;
+ }
+ if (ns->netdev->mtu > NSIM_XDP_MAX_MTU) {
+ NSIM_EA(bpf->extack, "MTU too large w/ XDP enabled");
+ return -EINVAL;
+ }
+ if (nsim_xdp_offload_active(ns)) {
+ NSIM_EA(bpf->extack, "xdp offload active, can't load drv prog");
+ return -EBUSY;
+ }
+ return 0;
+}
+
+static int
+nsim_setup_prog_hw_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
+{
+ struct nsim_bpf_bound_prog *state;
+
+ if (!bpf->prog)
+ return 0;
+
+ if (!bpf->prog->aux->offload) {
+ NSIM_EA(bpf->extack, "xdpoffload of non-bound program");
+ return -EINVAL;
+ }
+ if (bpf->prog->aux->offload->netdev != ns->netdev) {
+ NSIM_EA(bpf->extack, "program bound to different dev");
+ return -EINVAL;
+ }
+
+ state = bpf->prog->aux->offload->dev_priv;
+ if (WARN_ON(strcmp(state->state, "xlated"))) {
+ NSIM_EA(bpf->extack, "offloading program in bad state");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+ struct nsim_bpf_bound_prog *state;
+ int err;
+
+ ASSERT_RTNL();
+
+ switch (bpf->command) {
+ case BPF_OFFLOAD_VERIFIER_PREP:
+ if (!ns->bpf_bind_accept)
+ return -EOPNOTSUPP;
+
+ err = nsim_bpf_create_prog(ns, bpf->verifier.prog);
+ if (err)
+ return err;
+
+ bpf->verifier.ops = &nsim_bpf_analyzer_ops;
+ return 0;
+ case BPF_OFFLOAD_TRANSLATE:
+ state = bpf->offload.prog->aux->offload->dev_priv;
+
+ state->state = "xlated";
+ return 0;
+ case BPF_OFFLOAD_DESTROY:
+ nsim_bpf_destroy_prog(bpf->offload.prog);
+ return 0;
+ case XDP_QUERY_PROG:
+ bpf->prog_attached = ns->xdp_prog_mode;
+ bpf->prog_id = ns->xdp_prog ? ns->xdp_prog->aux->id : 0;
+ bpf->prog_flags = ns->xdp_prog ? ns->xdp_flags : 0;
+ return 0;
+ case XDP_SETUP_PROG:
+ err = nsim_setup_prog_checks(ns, bpf);
+ if (err)
+ return err;
+
+ return nsim_xdp_set_prog(ns, bpf);
+ case XDP_SETUP_PROG_HW:
+ err = nsim_setup_prog_hw_checks(ns, bpf);
+ if (err)
+ return err;
+
+ return nsim_xdp_set_prog(ns, bpf);
+ default:
+ return -EINVAL;
+ }
+}
+
+int nsim_bpf_init(struct netdevsim *ns)
+{
+ INIT_LIST_HEAD(&ns->bpf_bound_progs);
+
+ debugfs_create_u32("bpf_offloaded_id", 0400, ns->ddir,
+ &ns->bpf_offloaded_id);
+
+ ns->bpf_bind_accept = true;
+ debugfs_create_bool("bpf_bind_accept", 0600, ns->ddir,
+ &ns->bpf_bind_accept);
+ debugfs_create_u32("bpf_bind_verifier_delay", 0600, ns->ddir,
+ &ns->bpf_bind_verifier_delay);
+ ns->ddir_bpf_bound_progs =
+ debugfs_create_dir("bpf_bound_progs", ns->ddir);
+
+ ns->bpf_tc_accept = true;
+ debugfs_create_bool("bpf_tc_accept", 0600, ns->ddir,
+ &ns->bpf_tc_accept);
+ debugfs_create_bool("bpf_tc_non_bound_accept", 0600, ns->ddir,
+ &ns->bpf_tc_non_bound_accept);
+ ns->bpf_xdpdrv_accept = true;
+ debugfs_create_bool("bpf_xdpdrv_accept", 0600, ns->ddir,
+ &ns->bpf_xdpdrv_accept);
+ ns->bpf_xdpoffload_accept = true;
+ debugfs_create_bool("bpf_xdpoffload_accept", 0600, ns->ddir,
+ &ns->bpf_xdpoffload_accept);
+
+ return 0;
+}
+
+void nsim_bpf_uninit(struct netdevsim *ns)
+{
+ WARN_ON(!list_empty(&ns->bpf_bound_progs));
+ WARN_ON(ns->xdp_prog);
+ WARN_ON(ns->bpf_offloaded);
+}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 7599c72c477a..828c1ce49a8b 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -13,16 +13,45 @@
* THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
*/
+#include <linux/debugfs.h>
#include <linux/etherdevice.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/netdevice.h>
#include <linux/slab.h>
#include <net/netlink.h>
+#include <net/pkt_cls.h>
#include <net/rtnetlink.h>
#include "netdevsim.h"
+static int nsim_init(struct net_device *dev)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+ int err;
+
+ ns->netdev = dev;
+ ns->ddir = debugfs_create_dir(netdev_name(dev), nsim_ddir);
+
+ err = nsim_bpf_init(ns);
+ if (err)
+ goto err_debugfs_destroy;
+
+ return 0;
+
+err_debugfs_destroy:
+ debugfs_remove_recursive(ns->ddir);
+ return err;
+}
+
+static void nsim_uninit(struct net_device *dev)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ debugfs_remove_recursive(ns->ddir);
+ nsim_bpf_uninit(ns);
+}
+
static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct netdevsim *ns = netdev_priv(dev);
@@ -41,6 +70,19 @@ static void nsim_set_rx_mode(struct net_device *dev)
{
}
+static int nsim_change_mtu(struct net_device *dev, int new_mtu)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (ns->xdp_prog_mode == XDP_ATTACHED_DRV &&
+ new_mtu > NSIM_XDP_MAX_MTU)
+ return -EBUSY;
+
+ dev->mtu = new_mtu;
+
+ return 0;
+}
+
static void
nsim_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
{
@@ -54,12 +96,66 @@ nsim_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
} while (u64_stats_fetch_retry(&ns->syncp, start));
}
+static int
+nsim_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv)
+{
+ return nsim_bpf_setup_tc_block_cb(type, type_data, cb_priv);
+}
+
+static int
+nsim_setup_tc_block(struct net_device *dev, struct tc_block_offload *f)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if (f->binder_type != TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
+ return -EOPNOTSUPP;
+
+ switch (f->command) {
+ case TC_BLOCK_BIND:
+ return tcf_block_cb_register(f->block, nsim_setup_tc_block_cb,
+ ns, ns);
+ case TC_BLOCK_UNBIND:
+ tcf_block_cb_unregister(f->block, nsim_setup_tc_block_cb, ns);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int
+nsim_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data)
+{
+ switch (type) {
+ case TC_SETUP_BLOCK:
+ return nsim_setup_tc_block(dev, type_data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int
+nsim_set_features(struct net_device *dev, netdev_features_t features)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ if ((dev->features & NETIF_F_HW_TC) > (features & NETIF_F_HW_TC))
+ return nsim_bpf_disable_tc(ns);
+
+ return 0;
+}
+
static const struct net_device_ops nsim_netdev_ops = {
+ .ndo_init = nsim_init,
+ .ndo_uninit = nsim_uninit,
.ndo_start_xmit = nsim_start_xmit,
.ndo_set_rx_mode = nsim_set_rx_mode,
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
+ .ndo_change_mtu = nsim_change_mtu,
.ndo_get_stats64 = nsim_get_stats64,
+ .ndo_setup_tc = nsim_setup_tc,
+ .ndo_set_features = nsim_set_features,
+ .ndo_bpf = nsim_bpf,
};
static void nsim_setup(struct net_device *dev)
@@ -80,6 +176,7 @@ static void nsim_setup(struct net_device *dev)
NETIF_F_FRAGLIST |
NETIF_F_HW_CSUM |
NETIF_F_TSO;
+ dev->hw_features |= NETIF_F_HW_TC;
dev->max_mtu = ETH_MAX_MTU;
}
@@ -102,14 +199,31 @@ static struct rtnl_link_ops nsim_link_ops __read_mostly = {
.validate = nsim_validate,
};
+struct dentry *nsim_ddir;
+
static int __init nsim_module_init(void)
{
- return rtnl_link_register(&nsim_link_ops);
+ int err;
+
+ nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
+ if (IS_ERR(nsim_ddir))
+ return PTR_ERR(nsim_ddir);
+
+ err = rtnl_link_register(&nsim_link_ops);
+ if (err)
+ goto err_debugfs_destroy;
+
+ return 0;
+
+err_debugfs_destroy:
+ debugfs_remove_recursive(nsim_ddir);
+ return err;
}
static void __exit nsim_module_exit(void)
{
rtnl_link_unregister(&nsim_link_ops);
+ debugfs_remove_recursive(nsim_ddir);
}
module_init(nsim_module_init);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 4558c6f11598..8779e6a8f885 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -14,13 +14,53 @@
*/
#include <linux/kernel.h>
+#include <linux/list.h>
#include <linux/netdevice.h>
#include <linux/u64_stats_sync.h>
#define DRV_NAME "netdevsim"
+#define NSIM_XDP_MAX_MTU 4000
+
+#define NSIM_EA(extack, msg) NL_SET_ERR_MSG_MOD((extack), msg)
+
+struct bpf_prog;
+struct dentry;
+
struct netdevsim {
+ struct net_device *netdev;
+
u64 tx_packets;
u64 tx_bytes;
struct u64_stats_sync syncp;
+
+ struct dentry *ddir;
+
+ struct bpf_prog *bpf_offloaded;
+ u32 bpf_offloaded_id;
+
+ u32 xdp_flags;
+ int xdp_prog_mode;
+ struct bpf_prog *xdp_prog;
+
+ u32 prog_id_gen;
+
+ bool bpf_bind_accept;
+ u32 bpf_bind_verifier_delay;
+ struct dentry *ddir_bpf_bound_progs;
+ struct list_head bpf_bound_progs;
+
+ bool bpf_tc_accept;
+ bool bpf_tc_non_bound_accept;
+ bool bpf_xdpdrv_accept;
+ bool bpf_xdpoffload_accept;
};
+
+extern struct dentry *nsim_ddir;
+
+int nsim_bpf_init(struct netdevsim *ns);
+void nsim_bpf_uninit(struct netdevsim *ns);
+int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf);
+int nsim_bpf_disable_tc(struct netdevsim *ns);
+int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
+ void *type_data, void *cb_priv);
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 4/8] netdevsim: add software driver for testing offloads
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev; +Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
To be able to run selftests without any hardware required we
need a software model. The model can also serve as an example
implementation for those implementing actual HW offloads.
The dummy driver have previously been extended to test SR-IOV,
but the general consensus seems to be against adding further
features to it.
Add a new driver for purposes of software modelling only.
eBPF and SR-IOV will be added here shortly, others are invited
to further extend the driver with their offload models.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
MAINTAINERS | 5 ++
drivers/net/Kconfig | 11 ++++
drivers/net/Makefile | 1 +
drivers/net/netdevsim/Makefile | 6 ++
drivers/net/netdevsim/netdev.c | 118 ++++++++++++++++++++++++++++++++++++++
drivers/net/netdevsim/netdevsim.h | 26 +++++++++
6 files changed, 167 insertions(+)
create mode 100644 drivers/net/netdevsim/Makefile
create mode 100644 drivers/net/netdevsim/netdev.c
create mode 100644 drivers/net/netdevsim/netdevsim.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 77d819b458a9..010e46a38373 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9599,6 +9599,11 @@ NETWORKING [WIRELESS]
L: linux-wireless@vger.kernel.org
Q: http://patchwork.kernel.org/project/linux-wireless/list/
+NETDEVSIM
+M: Jakub Kicinski <jakub.kicinski@netronome.com>
+S: Maintained
+F: drivers/net/netdevsim/*
+
NETXEN (1/10) GbE SUPPORT
M: Manish Chopra <manish.chopra@cavium.com>
M: Rahul Verma <rahul.verma@cavium.com>
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 0936da592e12..944ec3c9282c 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -497,4 +497,15 @@ config THUNDERBOLT_NET
source "drivers/net/hyperv/Kconfig"
+config NETDEVSIM
+ tristate "Simulated networking device"
+ depends on DEBUG_FS
+ help
+ This driver is a developer testing tool and software model that can
+ be used to test various control path networking APIs, especially
+ HW-offload related.
+
+ To compile this driver as a module, choose M here: the module
+ will be called netdevsim.
+
endif # NETDEVICES
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 766f62d02a0b..04c3b747812c 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -78,3 +78,4 @@ obj-$(CONFIG_FUJITSU_ES) += fjes/
thunderbolt-net-y += thunderbolt.o
obj-$(CONFIG_THUNDERBOLT_NET) += thunderbolt-net.o
+obj-$(CONFIG_NETDEVSIM) += netdevsim/
diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
new file mode 100644
index 000000000000..07867bfe873b
--- /dev/null
+++ b/drivers/net/netdevsim/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_NETDEVSIM) += netdevsim.o
+
+netdevsim-objs := \
+ netdev.o \
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
new file mode 100644
index 000000000000..7599c72c477a
--- /dev/null
+++ b/drivers/net/netdevsim/netdev.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2017 Netronome Systems, Inc.
+ *
+ * This software is licensed under the GNU General License Version 2,
+ * June 1991 as shown in the file COPYING in the top-level directory of this
+ * source tree.
+ *
+ * THE COPYRIGHT HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS"
+ * WITHOUT WARRANTY OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE
+ * OF THE PROGRAM IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME
+ * THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
+ */
+
+#include <linux/etherdevice.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/slab.h>
+#include <net/netlink.h>
+#include <net/rtnetlink.h>
+
+#include "netdevsim.h"
+
+static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+
+ u64_stats_update_begin(&ns->syncp);
+ ns->tx_packets++;
+ ns->tx_bytes += skb->len;
+ u64_stats_update_end(&ns->syncp);
+
+ dev_kfree_skb(skb);
+
+ return NETDEV_TX_OK;
+}
+
+static void nsim_set_rx_mode(struct net_device *dev)
+{
+}
+
+static void
+nsim_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+ unsigned int start;
+
+ do {
+ start = u64_stats_fetch_begin(&ns->syncp);
+ stats->tx_bytes = ns->tx_bytes;
+ stats->tx_packets = ns->tx_packets;
+ } while (u64_stats_fetch_retry(&ns->syncp, start));
+}
+
+static const struct net_device_ops nsim_netdev_ops = {
+ .ndo_start_xmit = nsim_start_xmit,
+ .ndo_set_rx_mode = nsim_set_rx_mode,
+ .ndo_set_mac_address = eth_mac_addr,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_get_stats64 = nsim_get_stats64,
+};
+
+static void nsim_setup(struct net_device *dev)
+{
+ ether_setup(dev);
+ eth_hw_addr_random(dev);
+
+ dev->netdev_ops = &nsim_netdev_ops;
+ dev->needs_free_netdev = true;
+
+ dev->tx_queue_len = 0;
+ dev->flags |= IFF_NOARP;
+ dev->flags &= ~IFF_MULTICAST;
+ dev->priv_flags |= IFF_LIVE_ADDR_CHANGE |
+ IFF_NO_QUEUE;
+ dev->features |= NETIF_F_HIGHDMA |
+ NETIF_F_SG |
+ NETIF_F_FRAGLIST |
+ NETIF_F_HW_CSUM |
+ NETIF_F_TSO;
+ dev->max_mtu = ETH_MAX_MTU;
+}
+
+static int nsim_validate(struct nlattr *tb[], struct nlattr *data[],
+ struct netlink_ext_ack *extack)
+{
+ if (tb[IFLA_ADDRESS]) {
+ if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+ return -EINVAL;
+ if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+ return -EADDRNOTAVAIL;
+ }
+ return 0;
+}
+
+static struct rtnl_link_ops nsim_link_ops __read_mostly = {
+ .kind = DRV_NAME,
+ .priv_size = sizeof(struct netdevsim),
+ .setup = nsim_setup,
+ .validate = nsim_validate,
+};
+
+static int __init nsim_module_init(void)
+{
+ return rtnl_link_register(&nsim_link_ops);
+}
+
+static void __exit nsim_module_exit(void)
+{
+ rtnl_link_unregister(&nsim_link_ops);
+}
+
+module_init(nsim_module_init);
+module_exit(nsim_module_exit);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_RTNL_LINK(DRV_NAME);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
new file mode 100644
index 000000000000..4558c6f11598
--- /dev/null
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2017 Netronome Systems, Inc.
+ *
+ * This software is licensed under the GNU General License Version 2,
+ * June 1991 as shown in the file COPYING in the top-level directory of this
+ * source tree.
+ *
+ * THE COPYRIGHT HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS"
+ * WITHOUT WARRANTY OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE
+ * OF THE PROGRAM IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME
+ * THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
+ */
+
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/u64_stats_sync.h>
+
+#define DRV_NAME "netdevsim"
+
+struct netdevsim {
+ u64 tx_packets;
+ u64 tx_bytes;
+ struct u64_stats_sync syncp;
+};
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 3/8] net: xdp: make the stack take care of the tear down
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev
Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski, Saeed Mahameed, Michael Chan, Ariel Elior,
John Fastabend
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
Since day one of XDP drivers had to remember to free the program
on the remove path. This leads to code duplication and is error
prone. Make the stack query the installed programs on unregister
and if something is installed, remove the program. Freeing of
program attached to XDP generic is moved from free_netdev() as well.
Because the remove will now be called before notifiers are
invoked, BPF offload state of the program will not get destroyed
before uninstall.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
CC: Saeed Mahameed <saeedm@mellanox.com>
CC: Michael Chan <michael.chan@broadcom.com>
CC: Ariel Elior <Ariel.Elior@cavium.com>
CC: John Fastabend <john.fastabend@gmail.com>
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 --
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 ---
drivers/net/ethernet/netronome/nfp/bpf/main.c | 7 ------
.../net/ethernet/netronome/nfp/nfp_net_common.c | 3 ---
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 ---
drivers/net/tun.c | 4 ---
net/core/dev.c | 29 ++++++++++++++++------
7 files changed, 22 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 33c49ad697e4..413ad2444ba2 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7800,8 +7800,6 @@ static void bnxt_remove_one(struct pci_dev *pdev)
bnxt_dcb_free(bp);
kfree(bp->edev);
bp->edev = NULL;
- if (bp->xdp_prog)
- bpf_prog_put(bp->xdp_prog);
bnxt_cleanup_pci(bp);
free_netdev(dev);
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index d2b057a3e512..0f5c012de52e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4308,9 +4308,6 @@ static void mlx5e_nic_cleanup(struct mlx5e_priv *priv)
{
mlx5e_ipsec_cleanup(priv);
mlx5e_vxlan_cleanup(priv);
-
- if (priv->channels.params.xdp_prog)
- bpf_prog_put(priv->channels.params.xdp_prog);
}
static int mlx5e_init_nic_rx(struct mlx5e_priv *priv)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index e379b78e86ef..54bfd7846f6d 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -82,12 +82,6 @@ static const char *nfp_bpf_extra_cap(struct nfp_app *app, struct nfp_net *nn)
return nfp_net_ebpf_capable(nn) ? "BPF" : "";
}
-static void nfp_bpf_vnic_free(struct nfp_app *app, struct nfp_net *nn)
-{
- if (nn->dp.bpf_offload_xdp)
- nfp_bpf_xdp_offload(app, nn, NULL);
-}
-
static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
void *type_data, void *cb_priv)
{
@@ -168,7 +162,6 @@ const struct nfp_app_type app_bpf = {
.extra_cap = nfp_bpf_extra_cap,
.vnic_alloc = nfp_app_nic_vnic_alloc,
- .vnic_free = nfp_bpf_vnic_free,
.setup_tc = nfp_bpf_setup_tc,
.tc_busy = nfp_bpf_tc_busy,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index ea6bbf1efefc..ad3e9f6a61e5 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3562,9 +3562,6 @@ struct nfp_net *nfp_net_alloc(struct pci_dev *pdev, bool needs_netdev,
*/
void nfp_net_free(struct nfp_net *nn)
{
- if (nn->xdp_prog)
- bpf_prog_put(nn->xdp_prog);
-
if (nn->dp.netdev)
free_netdev(nn->dp.netdev);
else
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 8f9b3eb82137..57332b3e5e64 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -1068,10 +1068,6 @@ static void __qede_remove(struct pci_dev *pdev, enum qede_remove_mode mode)
pci_set_drvdata(pdev, NULL);
- /* Release edev's reference to XDP's bpf if such exist */
- if (edev->xdp_prog)
- bpf_prog_put(edev->xdp_prog);
-
/* Use global ops since we've freed edev */
qed_ops->common->slowpath_stop(cdev);
if (system_state == SYSTEM_POWER_OFF)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 6a7bde9bc4b2..6f7e8e45c961 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -673,7 +673,6 @@ static void tun_detach(struct tun_file *tfile, bool clean)
static void tun_detach_all(struct net_device *dev)
{
struct tun_struct *tun = netdev_priv(dev);
- struct bpf_prog *xdp_prog = rtnl_dereference(tun->xdp_prog);
struct tun_file *tfile, *tmp;
int i, n = tun->numqueues;
@@ -708,9 +707,6 @@ static void tun_detach_all(struct net_device *dev)
}
BUG_ON(tun->numdisabled != 0);
- if (xdp_prog)
- bpf_prog_put(xdp_prog);
-
if (tun->flags & IFF_PERSIST)
module_put(THIS_MODULE);
}
diff --git a/net/core/dev.c b/net/core/dev.c
index 3f271c9cb5e0..6bea8931bb62 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7110,6 +7110,27 @@ static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
return bpf_op(dev, &xdp);
}
+static void dev_xdp_uninstall(struct net_device *dev)
+{
+ struct netdev_bpf xdp;
+ bpf_op_t ndo_bpf;
+
+ /* Remove generic XDP */
+ WARN_ON(dev_xdp_install(dev, generic_xdp_install, NULL, 0, NULL));
+
+ /* Remove from the driver */
+ ndo_bpf = dev->netdev_ops->ndo_bpf;
+ if (!ndo_bpf)
+ return;
+
+ __dev_xdp_query(dev, ndo_bpf, &xdp);
+ if (xdp.prog_attached == XDP_ATTACHED_NONE)
+ return;
+
+ /* Program removal should always succeed */
+ WARN_ON(dev_xdp_install(dev, ndo_bpf, NULL, xdp.prog_flags, NULL));
+}
+
/**
* dev_change_xdp_fd - set or clear a bpf program for a device rx path
* @dev: device
@@ -7240,6 +7261,7 @@ static void rollback_registered_many(struct list_head *head)
/* Shutdown queueing discipline. */
dev_shutdown(dev);
+ dev_xdp_uninstall(dev);
/* Notify protocols, that we are about to destroy
* this device. They should clean all the things.
@@ -8199,7 +8221,6 @@ EXPORT_SYMBOL(alloc_netdev_mqs);
void free_netdev(struct net_device *dev)
{
struct napi_struct *p, *n;
- struct bpf_prog *prog;
might_sleep();
netif_free_tx_queues(dev);
@@ -8218,12 +8239,6 @@ void free_netdev(struct net_device *dev)
free_percpu(dev->pcpu_refcnt);
dev->pcpu_refcnt = NULL;
- prog = rcu_dereference_protected(dev->xdp_prog, 1);
- if (prog) {
- bpf_prog_put(prog);
- static_key_slow_dec(&generic_xdp_needed);
- }
-
/* Compatibility with error handling in drivers */
if (dev->reg_state == NETREG_UNINITIALIZED) {
netdev_freemem(dev);
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 2/8] net: xdp: report flags program was installed with on query
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev; +Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
Some drivers enforce that flags on program replacement and
removal must match the flags passed on install. This leaves
the possibility open to enable simultaneous loading
of XDP programs both to HW and DRV.
Allow such drivers to report the flags back to the stack.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
include/linux/netdevice.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 1a603fdd9e80..ea6bbf1efefc 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3392,6 +3392,7 @@ static int nfp_net_xdp(struct net_device *netdev, struct netdev_bpf *xdp)
if (nn->dp.bpf_offload_xdp)
xdp->prog_attached = XDP_ATTACHED_HW;
xdp->prog_id = nn->xdp_prog ? nn->xdp_prog->aux->id : 0;
+ xdp->flags = nn->xdp_prog ? nn->xdp_flags : 0;
return 0;
case BPF_OFFLOAD_VERIFIER_PREP:
return nfp_app_bpf_verifier_prep(nn->app, nn, xdp);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 667bdd3ad33e..cc4ce7456e38 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -820,6 +820,8 @@ struct netdev_bpf {
struct {
u8 prog_attached;
u32 prog_id;
+ /* flags with which program was installed */
+ u32 prog_flags;
};
/* BPF_OFFLOAD_VERIFIER_PREP */
struct {
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 1/8] net: xdp: avoid output parameters when querying XDP prog
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev; +Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski
In-Reply-To: <20171130222853.29395-1-jakub.kicinski@netronome.com>
The output parameters will get unwieldy if we want to add more
information about the program. Simply pass the entire
struct netdev_bpf in.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
include/linux/netdevice.h | 3 ++-
net/core/dev.c | 24 ++++++++++++++----------
net/core/rtnetlink.c | 6 +++++-
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ef789e1d679e..667bdd3ad33e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3330,7 +3330,8 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
typedef int (*bpf_op_t)(struct net_device *dev, struct netdev_bpf *bpf);
int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
int fd, u32 flags);
-u8 __dev_xdp_attached(struct net_device *dev, bpf_op_t xdp_op, u32 *prog_id);
+void __dev_xdp_query(struct net_device *dev, bpf_op_t xdp_op,
+ struct netdev_bpf *xdp);
int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 07ed21d64f92..3f271c9cb5e0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7073,17 +7073,21 @@ int dev_change_proto_down(struct net_device *dev, bool proto_down)
}
EXPORT_SYMBOL(dev_change_proto_down);
-u8 __dev_xdp_attached(struct net_device *dev, bpf_op_t bpf_op, u32 *prog_id)
+void __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
+ struct netdev_bpf *xdp)
{
- struct netdev_bpf xdp;
-
- memset(&xdp, 0, sizeof(xdp));
- xdp.command = XDP_QUERY_PROG;
+ memset(xdp, 0, sizeof(*xdp));
+ xdp->command = XDP_QUERY_PROG;
/* Query must always succeed. */
- WARN_ON(bpf_op(dev, &xdp) < 0);
- if (prog_id)
- *prog_id = xdp.prog_id;
+ WARN_ON(bpf_op(dev, xdp) < 0);
+}
+
+static u8 __dev_xdp_attached(struct net_device *dev, bpf_op_t bpf_op)
+{
+ struct netdev_bpf xdp;
+
+ __dev_xdp_query(dev, bpf_op, &xdp);
return xdp.prog_attached;
}
@@ -7134,10 +7138,10 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
bpf_chk = generic_xdp_install;
if (fd >= 0) {
- if (bpf_chk && __dev_xdp_attached(dev, bpf_chk, NULL))
+ if (bpf_chk && __dev_xdp_attached(dev, bpf_chk))
return -EEXIST;
if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) &&
- __dev_xdp_attached(dev, bpf_op, NULL))
+ __dev_xdp_attached(dev, bpf_op))
return -EBUSY;
prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index dabba2a91fc8..9c4cb584bfb0 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1261,6 +1261,7 @@ static u8 rtnl_xdp_attached_mode(struct net_device *dev, u32 *prog_id)
{
const struct net_device_ops *ops = dev->netdev_ops;
const struct bpf_prog *generic_xdp_prog;
+ struct netdev_bpf xdp;
ASSERT_RTNL();
@@ -1273,7 +1274,10 @@ static u8 rtnl_xdp_attached_mode(struct net_device *dev, u32 *prog_id)
if (!ops->ndo_bpf)
return XDP_ATTACHED_NONE;
- return __dev_xdp_attached(dev, ops->ndo_bpf, prog_id);
+ __dev_xdp_query(dev, ops->ndo_bpf, &xdp);
+ *prog_id = xdp.prog_id;
+
+ return xdp.prog_attached;
}
static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 0/8] xdp: make stack perform remove and tests
From: Jakub Kicinski @ 2017-11-30 22:28 UTC (permalink / raw)
To: netdev; +Cc: davem, daniel, alexei.starovoitov, jiri, oss-drivers,
Jakub Kicinski
Hi!
The purpose of this series is to add a software model of BPF offloads
to make it easier for everyone to test them and make some of the more
arcane rules and assumptions more clear.
The series starts with 3 patches aiming to make XDP handling in the
drivers less error prone. Currently driver authors have to remember
to free XDP programs if XDP is active during unregister. With this
series the core will disable XDP on its own. It will take place
after close, drivers are not expected to perform reconfiguration
when disabling XDP on a downed device.
Next two patches add the software netdev driver, followed by a python
test which exercises all the corner cases which came to my mind.
Test needs to be run as root. It will print basic information to
stdout, but can also create a more detailed log of all commands
when --log option is passed. Log is in Emacs Org-mode format.
./tools/testing/selftests/bpf/test_offload.py --log /tmp/log
Last two patches replace the SR-IOV API implementation of dummy.
v1:
- replace the SR-IOV API implementation of dummy;
- make the dev_xdp_uninstall() also handle the XDP generic (Daniel).
Jakub Kicinski (8):
net: xdp: avoid output parameters when querying XDP prog
net: xdp: report flags program was installed with on query
net: xdp: make the stack take care of the tear down
netdevsim: add software driver for testing offloads
netdevsim: add bpf offload support
selftests/bpf: add offload test based on netdevsim
netdevsim: add SR-IOV functionality
net: dummy: remove fake SR-IOV functionality
MAINTAINERS | 5 +
drivers/net/Kconfig | 11 +
drivers/net/Makefile | 1 +
drivers/net/dummy.c | 215 +------
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 -
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 -
drivers/net/ethernet/netronome/nfp/bpf/main.c | 7 -
.../net/ethernet/netronome/nfp/nfp_net_common.c | 4 +-
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 -
drivers/net/netdevsim/Makefile | 7 +
drivers/net/netdevsim/bpf.c | 373 +++++++++++
drivers/net/netdevsim/netdev.c | 512 ++++++++++++++++
drivers/net/netdevsim/netdevsim.h | 75 +++
drivers/net/tun.c | 4 -
include/linux/netdevice.h | 5 +-
net/core/dev.c | 53 +-
net/core/rtnetlink.c | 6 +-
tools/testing/selftests/bpf/Makefile | 5 +-
tools/testing/selftests/bpf/sample_ret0.c | 7 +
tools/testing/selftests/bpf/test_offload.py | 681 +++++++++++++++++++++
20 files changed, 1722 insertions(+), 258 deletions(-)
create mode 100644 drivers/net/netdevsim/Makefile
create mode 100644 drivers/net/netdevsim/bpf.c
create mode 100644 drivers/net/netdevsim/netdev.c
create mode 100644 drivers/net/netdevsim/netdevsim.h
create mode 100644 tools/testing/selftests/bpf/sample_ret0.c
create mode 100755 tools/testing/selftests/bpf/test_offload.py
--
2.14.1
^ permalink raw reply
* Re: [PATCH/RFC] Re: 'perf test BPF' failing, libbpf regression wrt "basic API for BPF obj name"
From: Alexei Starovoitov @ 2017-11-30 21:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Martin KaFai Lau
Cc: Wang Nan, Daniel Borkmann, David S. Miller, David Ahern,
Ingo Molnar, Linux Kernel Mailing List, netdev
In-Reply-To: <20171130190042.GQ3298@kernel.org>
On 11/30/17 11:00 AM, Arnaldo Carvalho de Melo wrote:
>> Instead of sinking all future bpf_attr's backward compatibility
>> requirements to sys_bpf, I would push it up to its own BPF_* command
>> helper which has a better sense of its bpf_attr, i.e. push it up
>> to bpf_create_map_node() and bpf_load_program_name() in this case.
> Humm, we could try that approach, but the one in this patch seemed good
> enough.
>
> And after all if the first syscall() invokation, with the latest kernel
> and latest tooling will work, right?
I agree with Martin and I also don't think it will work to push
logic of all bpf commands into single sys_bpf syscall wrapper.
This logic will become more and more complex over time.
Like this case really belongs in bpf_create_map() which is a wrapper
on top of single BPF_CREATE_MAP command.
Note it's the first time we're facing this
'new libbpf.a running on top of old kernel' issue and should be
very careful adding such fallback code to the generic bpf library,
since all the selftests/bpf/ are using this lib and relying on
excepted behavior. We don't want tests that want to test the latest
kernel feature all of a sudden pass on old kernel that doesn't have it.
To some degree perf and selftests/bpf needs are diverging here,
so adding #ifdef to libbpf.a to match testcase expectations may be
necessary.
^ permalink raw reply
* [PATCH net 2/2] samples/bpf: add error checking for perf ioctl calls in bpf loader
From: Yonghong Song @ 2017-11-30 21:47 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20171130214755.3022117-1-yhs@fb.com>
load_bpf_file() should fail if ioctl with command
PERF_EVENT_IOC_ENABLE and PERF_EVENT_IOC_SET_BPF fails.
When they do fail, proper error messages are printed.
With this change, the below "syscall_tp" run shows that
the maximum number of bpf progs attaching to the same
perf tracepoint is indeed enforced.
$ ./syscall_tp -i 64
prog #0: map ids 4 5
...
prog #63: map ids 382 383
$ ./syscall_tp -i 65
prog #0: map ids 4 5
...
prog #64: map ids 388 389
ioctl PERF_EVENT_IOC_SET_BPF failed err Argument list too long
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/bpf_load.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 522ca92..242631a 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -193,8 +193,18 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
return -1;
}
event_fd[prog_cnt - 1] = efd;
- ioctl(efd, PERF_EVENT_IOC_ENABLE, 0);
- ioctl(efd, PERF_EVENT_IOC_SET_BPF, fd);
+ err = ioctl(efd, PERF_EVENT_IOC_ENABLE, 0);
+ if (err < 0) {
+ printf("ioctl PERF_EVENT_IOC_ENABLE failed err %s\n",
+ strerror(errno));
+ return -1;
+ }
+ err = ioctl(efd, PERF_EVENT_IOC_SET_BPF, fd);
+ if (err < 0) {
+ printf("ioctl PERF_EVENT_IOC_SET_BPF failed err %s\n",
+ strerror(errno));
+ return -1;
+ }
return 0;
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.
From: David Daney @ 2017-11-30 21:49 UTC (permalink / raw)
To: James Hogan, David Daney
Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, ralf-6z/3iImG2C8G8FEW9MqTrA,
netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, Rob Herring,
Mark Rutland, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Steven J. Hill, devicetree-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn,
Florian Fainelli, Carlos Munoz
In-Reply-To: <20171130213635.GH27409-4bYivNCBEGSP4qXr0kR+DFHK5/nzsB32@public.gmane.org>
On 11/30/2017 01:36 PM, James Hogan wrote:
> On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote:
>> From: Carlos Munoz <cmunoz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>
>> LMTDMA/LMTST operations move data between cores and I/O devices:
>>
>> * LMTST operations can send an address and a variable length
>> (up to 128 bytes) of data to an I/O device.
>> * LMTDMA operations can send an address and a variable length
>> (up to 128) of data to the I/O device and then return a
>> variable length (up to 128 bytes) response from the IOI device.
>
> Should that be "I/O"?
Yes, I will fix the changelog.
>
>>
>> Signed-off-by: Carlos Munoz <cmunoz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Steven J. Hill <Steven.Hill-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> ---
>> arch/mips/cavium-octeon/setup.c | 6 ++++++
>> arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++--
>> 2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c
>> index a8034d0dcade..99e6a68bc652 100644
>> --- a/arch/mips/cavium-octeon/setup.c
>> +++ b/arch/mips/cavium-octeon/setup.c
>> @@ -609,6 +609,12 @@ void octeon_user_io_init(void)
>> #else
>> cvmmemctl.s.cvmsegenak = 0;
>> #endif
>> + if (OCTEON_IS_OCTEON3()) {
>> + /* Enable LMTDMA */
>> + cvmmemctl.s.lmtena = 1;
>> + /* Scratch line to use for LMT operation */
>> + cvmmemctl.s.lmtline = 2;
>
> Out of curiosity, is there significance to the value 2 and associated
> virtual address 0xffffffffffff8100, or is it pretty arbitrary?
Yes, there is significance.
CPU local memory starts at 0xffffffffffff8000, each line is 0x80 bytes.
so the 2nd line starts at 0xffffffffffff8100
>
>> + }
>> /* R/W If set, CVMSEG is available for loads/stores in
>> * supervisor mode. */
>> cvmmemctl.s.cvmsegenas = 0;
>> diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h
>> index c99c4b6a79f4..92a17d67c1fa 100644
>> --- a/arch/mips/include/asm/octeon/octeon.h
>> +++ b/arch/mips/include/asm/octeon/octeon.h
>> @@ -179,7 +179,15 @@ union octeon_cvmemctl {
>> /* RO 1 = BIST fail, 0 = BIST pass */
>> __BITFIELD_FIELD(uint64_t wbfbist:1,
>> /* Reserved */
>> - __BITFIELD_FIELD(uint64_t reserved:17,
>> + __BITFIELD_FIELD(uint64_t reserved_52_57:6,
>> + /* When set, LMTDMA/LMTST operations are permitted */
>> + __BITFIELD_FIELD(uint64_t lmtena:1,
>> + /* Selects the CVMSEG LM cacheline used by LMTDMA
>> + * LMTST and wide atomic store operations.
>> + */
>> + __BITFIELD_FIELD(uint64_t lmtline:6,
>> + /* Reserved */
>> + __BITFIELD_FIELD(uint64_t reserved_41_44:4,
>> /* OCTEON II - TLB replacement policy: 0 = bitmask LRU; 1 = NLU.
>> * This field selects between the TLB replacement policies:
>> * bitmask LRU or NLU. Bitmask LRU maintains a mask of
>> @@ -275,7 +283,7 @@ union octeon_cvmemctl {
>> /* R/W Size of local memory in cache blocks, 54 (6912
>> * bytes) is max legal value. */
>> __BITFIELD_FIELD(uint64_t lmemsz:6,
>> - ;)))))))))))))))))))))))))))))))))
>> + ;))))))))))))))))))))))))))))))))))))
>> } s;
>> };
>
> Regardless, the patch looks good to me.
>
> Reviewed-by: James Hogan <jhogan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Cheers
> James
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net 1/2] bpf: set maximum number of attached progs to 64 for a single perf tp
From: Yonghong Song @ 2017-11-30 21:47 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
cgropu+bpf prog array has a maximum number of 64 programs.
Let us apply the same limit here.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/core.c | 3 ++-
kernel/trace/bpf_trace.c | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index b9f8686..86b50aa 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1447,7 +1447,8 @@ int bpf_prog_array_length(struct bpf_prog_array __rcu *progs)
rcu_read_lock();
prog = rcu_dereference(progs)->progs;
for (; *prog; prog++)
- cnt++;
+ if (*prog != &dummy_bpf_prog.prog)
+ cnt++;
rcu_read_unlock();
return cnt;
}
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 27d1f4f..0ce99c3 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -759,6 +759,8 @@ const struct bpf_prog_ops perf_event_prog_ops = {
static DEFINE_MUTEX(bpf_event_mutex);
+#define BPF_TRACE_MAX_PROGS 64
+
int perf_event_attach_bpf_prog(struct perf_event *event,
struct bpf_prog *prog)
{
@@ -772,6 +774,12 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
goto unlock;
old_array = event->tp_event->prog_array;
+ if (old_array &&
+ bpf_prog_array_length(old_array) >= BPF_TRACE_MAX_PROGS) {
+ ret = -E2BIG;
+ goto unlock;
+ }
+
ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
if (ret < 0)
goto unlock;
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.
From: James Hogan @ 2017-11-30 21:36 UTC (permalink / raw)
To: David Daney
Cc: Mark Rutland, linux-mips, devel, devicetree, netdev, linux-kernel,
ralf, Carlos Munoz, Rob Herring, Andrew Lunn, Steven J. Hill,
Greg Kroah-Hartman, Florian Fainelli, David S. Miller
In-Reply-To: <20171129005540.28829-3-david.daney@cavium.com>
[-- Attachment #1.1: Type: text/plain, Size: 3023 bytes --]
On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote:
> From: Carlos Munoz <cmunoz@cavium.com>
>
> LMTDMA/LMTST operations move data between cores and I/O devices:
>
> * LMTST operations can send an address and a variable length
> (up to 128 bytes) of data to an I/O device.
> * LMTDMA operations can send an address and a variable length
> (up to 128) of data to the I/O device and then return a
> variable length (up to 128 bytes) response from the IOI device.
Should that be "I/O"?
>
> Signed-off-by: Carlos Munoz <cmunoz@cavium.com>
> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
> arch/mips/cavium-octeon/setup.c | 6 ++++++
> arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++--
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c
> index a8034d0dcade..99e6a68bc652 100644
> --- a/arch/mips/cavium-octeon/setup.c
> +++ b/arch/mips/cavium-octeon/setup.c
> @@ -609,6 +609,12 @@ void octeon_user_io_init(void)
> #else
> cvmmemctl.s.cvmsegenak = 0;
> #endif
> + if (OCTEON_IS_OCTEON3()) {
> + /* Enable LMTDMA */
> + cvmmemctl.s.lmtena = 1;
> + /* Scratch line to use for LMT operation */
> + cvmmemctl.s.lmtline = 2;
Out of curiosity, is there significance to the value 2 and associated
virtual address 0xffffffffffff8100, or is it pretty arbitrary?
> + }
> /* R/W If set, CVMSEG is available for loads/stores in
> * supervisor mode. */
> cvmmemctl.s.cvmsegenas = 0;
> diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h
> index c99c4b6a79f4..92a17d67c1fa 100644
> --- a/arch/mips/include/asm/octeon/octeon.h
> +++ b/arch/mips/include/asm/octeon/octeon.h
> @@ -179,7 +179,15 @@ union octeon_cvmemctl {
> /* RO 1 = BIST fail, 0 = BIST pass */
> __BITFIELD_FIELD(uint64_t wbfbist:1,
> /* Reserved */
> - __BITFIELD_FIELD(uint64_t reserved:17,
> + __BITFIELD_FIELD(uint64_t reserved_52_57:6,
> + /* When set, LMTDMA/LMTST operations are permitted */
> + __BITFIELD_FIELD(uint64_t lmtena:1,
> + /* Selects the CVMSEG LM cacheline used by LMTDMA
> + * LMTST and wide atomic store operations.
> + */
> + __BITFIELD_FIELD(uint64_t lmtline:6,
> + /* Reserved */
> + __BITFIELD_FIELD(uint64_t reserved_41_44:4,
> /* OCTEON II - TLB replacement policy: 0 = bitmask LRU; 1 = NLU.
> * This field selects between the TLB replacement policies:
> * bitmask LRU or NLU. Bitmask LRU maintains a mask of
> @@ -275,7 +283,7 @@ union octeon_cvmemctl {
> /* R/W Size of local memory in cache blocks, 54 (6912
> * bytes) is max legal value. */
> __BITFIELD_FIELD(uint64_t lmemsz:6,
> - ;)))))))))))))))))))))))))))))))))
> + ;))))))))))))))))))))))))))))))))))))
> } s;
> };
Regardless, the patch looks good to me.
Reviewed-by: James Hogan <jhogan@kernel.org>
Cheers
James
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [PATCH] net: ethernet: ixp4xx_eth: Use dma_pool_zalloc
From: Gomonovych, Vasyl @ 2017-11-30 21:32 UTC (permalink / raw)
To: netdev, khalasa
In-Reply-To: <1512076939-32323-1-git-send-email-gomonovych@gmail.com>
Sorry, Ignore this please.
On Thu, Nov 30, 2017 at 10:22 PM, Vasyl Gomonovych <gomonovych@gmail.com> wrote:
> Replacing dma_pool_alloc and memset with a single call to dma_pool_zalloc
>
> Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com>
> ---
> drivers/net/ethernet/xscale/ixp4xx_eth.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
> index aee55c03def0..02f7f3e3da8d 100644
> --- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
> +++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
> @@ -1108,10 +1108,10 @@ static int init_queues(struct port *port)
> return -ENOMEM;
> }
>
> - if (!(port->desc_tab = dma_pool_alloc(dma_pool, GFP_KERNEL,
> - &port->desc_tab_phys)))
> + port->desc_tab = dma_pool_zalloc(dma_pool, GFP_KERNEL,
> + &port->desc_tab_phys);
> + if (!port->desc_tab)
> return -ENOMEM;
> - memset(port->desc_tab, 0, POOL_ALLOC_SIZE);
> memset(port->rx_buff_tab, 0, sizeof(port->rx_buff_tab)); /* tables */
> memset(port->tx_buff_tab, 0, sizeof(port->tx_buff_tab));
>
> --
> 1.9.1
>
--
Доброї вам пори дня.
^ permalink raw reply
* [PATCH] net: ethernet: ixp4xx_eth: Use dma_pool_zalloc
From: Vasyl Gomonovych @ 2017-11-30 21:22 UTC (permalink / raw)
To: netdev, khalasa, gomonovych; +Cc: linux-kernel
Replacing dma_pool_alloc and memset with a single call to dma_pool_zalloc
Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com>
---
drivers/net/ethernet/xscale/ixp4xx_eth.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index aee55c03def0..02f7f3e3da8d 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -1108,10 +1108,10 @@ static int init_queues(struct port *port)
return -ENOMEM;
}
- if (!(port->desc_tab = dma_pool_alloc(dma_pool, GFP_KERNEL,
- &port->desc_tab_phys)))
+ port->desc_tab = dma_pool_zalloc(dma_pool, GFP_KERNEL,
+ &port->desc_tab_phys);
+ if (!port->desc_tab)
return -ENOMEM;
- memset(port->desc_tab, 0, POOL_ALLOC_SIZE);
memset(port->rx_buff_tab, 0, sizeof(port->rx_buff_tab)); /* tables */
memset(port->tx_buff_tab, 0, sizeof(port->tx_buff_tab));
--
1.9.1
^ permalink raw reply related
* Re: Creating cyclecounter and lock member in timecounter structure [ Was Re: [RFC 1/4] drm/i915/perf: Add support to correlate GPU timestamp with system time]
From: Saeed Mahameed @ 2017-11-30 21:03 UTC (permalink / raw)
To: Sagar Arun Kamble
Cc: Thomas Gleixner, John Stultz, Stephen Boyd, Chris Wilson,
linux-kernel, Linux Netdev List,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <63a2a495-1bdb-5d47-1202-9b538e9601d8-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Mon, Nov 27, 2017 at 2:05 AM, Sagar Arun Kamble
<sagar.a.kamble-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
>
> On 11/24/2017 7:01 PM, Thomas Gleixner wrote:
>>
>> On Fri, 24 Nov 2017, Sagar Arun Kamble wrote:
>>>
>>> On 11/24/2017 12:29 AM, Thomas Gleixner wrote:
>>>>
>>>> On Thu, 23 Nov 2017, Sagar Arun Kamble wrote:
>>>>>
>>>>> We needed inputs on possible optimization that can be done to
>>>>> timecounter/cyclecounter structures/usage.
>>>>> This mail is in response to review of patch
>>>>> https://patchwork.freedesktop.org/patch/188448/.
>>>>>
>>>>> As Chris's observation below, about dozen of timecounter users in the
>>>>> kernel
>>>>> have below structures
>>>>> defined individually:
>>>>>
>>>>> spinlock_t lock;
>>>>> struct cyclecounter cc;
>>>>> struct timecounter tc;
>>>>>
>>>>> Can we move lock and cc to tc? That way it will be convenient.
>>>>> Also it will allow unifying the locking/overflow watchdog handling
>>>>> across
>>>>> all
>>>>> drivers.
>>>>
>>>> Looks like none of the timecounter usage sites has a real need to
>>>> separate
>>>> timecounter and cyclecounter.
>>>
>>> Yes. Will share patch for this change.
>>>
>>>> The lock is a different question. The locking of the various drivers
>>>> differs and I have no idea how you want to handle that. Just sticking
>>>> the
>>>> lock into the datastructure and then not making use of it in the
>>>> timercounter code and leave it to the callsites does not make sense.
>>>
>>> Most of the locks are held around timecounter_read. In some instances it
>>> is held when cyclecounter is updated standalone or is updated along with
>>> timecounter calls. Was thinking if we move the lock in timecounter
>>> functions, drivers just have to do locking around its operations on
>>> cyclecounter. But then another problem I see is there are variation of
>>> locking calls like lock_irqsave, lock_bh, write_lock_irqsave (some using
>>> rwlock_t). Should this all locking be left to driver only then?
>>
>> You could have the lock in the struct and protect the inner workings in
>> the
>> related core functions.
>>
>> That might remove locking requirements from some of the callers and the
>> others still have their own thing around it.
>
>
> For drivers having static/fixed cyclecounter, we can rely only on lock
> inside timecounter.
> Most of the network drivers update cyclecounter at runtime and they will
> have to rely on two locks if
> we add one to timecounter. This may not be efficient for them. Also the lock
> in timecounter has to be less restrictive (may be seqlock) I guess.
>
> Cc'd Mellanox list for inputs on this.
>
> I have started feeling that the current approach of drivers managing the
> locks is the right one so better leave the
> lock out of timecounter.
>
I agree here,
In mlx5 we rely on our own read/write lock to serialize access to
mlx5_clock struct (mlx5 timecounter and cyclecounter).
the access is not as simple as
lock()
call time_counter_API
unlock()
Sometimes we also explicitly update/adjust timecycles counters with
mlx5 specific calculations after we read the timecounter all inside
our lock.
e.g.
@mlx5_ptp_adjfreq()
write_lock_irqsave(&clock->lock, flags);
timecounter_read(&clock->tc);
clock->cycles.mult = neg_adj ? clock->nominal_c_mult - diff :
clock->nominal_c_mult + diff;
write_unlock_irqrestore(&clock->lock, flags);
So i don't think it will be a simple task to have a generic thread
safe timecounter API, without the need to specifically adjust it for
all driver use-cases.
Also as said above, in runtime it is not obvious in which context the
timecounter will be accessed irq/soft irq/user.
let's keep it as is, and let the driver decide which locking scheme is
most suitable for it.
Thanks,
Saeed.
>> Thanks,
>>
>> tglx
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC 0/2] veth, bridge, and GSO maximums
From: Alexander Duyck @ 2017-11-30 20:50 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, Netdev, sthemmin
In-Reply-To: <20171130091130.0400d8a5@xeon-e3>
On Thu, Nov 30, 2017 at 9:11 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Thu, 30 Nov 2017 10:47:21 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Stephen Hemminger <stephen@networkplumber.org>
>> Date: Sun, 26 Nov 2017 10:17:47 -0800
>>
>> > This pair of patchesimproves the performance when running
>> > containers in an environment where underlying device has lower
>> > GSO maximum (such as Azure).
>> >
>> > With containers a veth pair is created and one end is attached
>> > to the bridge device. The bridge device correctly reports
>> > computes GSO parameters that are the minimum of the lower devices.
>> >
>> > The problem is that the other end of the veth device (in container)
>> > reports the full GSO size. This patch propogates the upper
>> > (bridge device) parameters to the other end of the veth device.
>> >
>> > Please consider it as alternative to the sysfs GSO changes.
>>
>> I like this approach a lot, please resubmit this formally.
>
> Will do and add netif_needs_gso check as well.
Would it make sense to look at possibly moving something like this
over to being handled by something like GSO_PARTIAL? Essentially it is
the same type of issue where we are needing to split a TSO frame into
smaller TSO frames. We already did something like this for the GRO
with skb->fraglist case, I wonder if it wouldn't make sense to just
handle the oversized GSO the same way. It seems like you could just
split add a check against the size and tweak the mss at the start of
skb_segment and that should be about all you need to do to take care
of it.
If I am not mistaken we could probably just tweak the one line that
was computing "partial_segs" in skb_segment so that we did something
like "partial_segs = min(len, dev->gso_max_size) / mss".
This way we take care of the issue for things like GRO as well which
doesn't take the max size into account last I knew, and you would
still get most of the benefits of TSO.
- Alex
^ permalink raw reply
* Re: [RFC] virtio-net: help live migrate SR-IOV devices
From: Jakub Kicinski @ 2017-11-30 20:48 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Jesse Brandeburg, virtualization, Sridhar Samudrala,
Achiad, Peter Waskiewicz Jr, Singhai, Anjali, Andy Gospodarek,
Or Gerlitz, netdev, Hannes Frederic Sowa
In-Reply-To: <20171130153522-mutt-send-email-mst@kernel.org>
On Thu, 30 Nov 2017 15:54:40 +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 29, 2017 at 07:51:38PM -0800, Jakub Kicinski wrote:
> > On Thu, 30 Nov 2017 11:29:56 +0800, Jason Wang wrote:
> > > On 2017年11月29日 03:27, Jesse Brandeburg wrote:
> > > > Hi, I'd like to get some feedback on a proposal to enhance virtio-net
> > > > to ease configuration of a VM and that would enable live migration of
> > > > passthrough network SR-IOV devices.
> > > >
> > > > Today we have SR-IOV network devices (VFs) that can be passed into a VM
> > > > in order to enable high performance networking direct within the VM.
> > > > The problem I am trying to address is that this configuration is
> > > > generally difficult to live-migrate. There is documentation [1]
> > > > indicating that some OS/Hypervisor vendors will support live migration
> > > > of a system with a direct assigned networking device. The problem I
> > > > see with these implementations is that the network configuration
> > > > requirements that are passed on to the owner of the VM are quite
> > > > complicated. You have to set up bonding, you have to configure it to
> > > > enslave two interfaces, those interfaces (one is virtio-net, the other
> > > > is SR-IOV device/driver like ixgbevf) must support MAC address changes
> > > > requested in the VM, and on and on...
> > > >
> > > > So, on to the proposal:
> > > > Modify virtio-net driver to be a single VM network device that
> > > > enslaves an SR-IOV network device (inside the VM) with the same MAC
> > > > address. This would cause the virtio-net driver to appear and work like
> > > > a simplified bonding/team driver. The live migration problem would be
> > > > solved just like today's bonding solution, but the VM user's networking
> > > > config would be greatly simplified.
> > > >
> > > > At it's simplest, it would appear something like this in the VM.
> > > >
> > > > ==========
> > > > = vnet0 =
> > > > =============
> > > > (virtio- = |
> > > > net) = |
> > > > = ==========
> > > > = = ixgbef =
> > > > ========== ==========
> > > >
> > > > (forgive the ASCII art)
> > > >
> > > > The fast path traffic would prefer the ixgbevf or other SR-IOV device
> > > > path, and fall back to virtio's transmit/receive when migrating.
> > > >
> > > > Compared to today's options this proposal would
> > > > 1) make virtio-net more sticky, allow fast path traffic at SR-IOV
> > > > speeds
> > > > 2) simplify end user configuration in the VM (most if not all of the
> > > > set up to enable migration would be done in the hypervisor)
> > > > 3) allow live migration via a simple link down and maybe a PCI
> > > > hot-unplug of the SR-IOV device, with failover to the virtio-net
> > > > driver core
> > > > 4) allow vendor agnostic hardware acceleration, and live migration
> > > > between vendors if the VM os has driver support for all the required
> > > > SR-IOV devices.
> > > >
> > > > Runtime operation proposed:
> > > > - <in either order> virtio-net driver loads, SR-IOV driver loads
> > > > - virtio-net finds other NICs that match it's MAC address by
> > > > both examining existing interfaces, and sets up a new device notifier
> > > > - virtio-net enslaves the first NIC with the same MAC address
> > > > - virtio-net brings up the slave, and makes it the "preferred" path
> > > > - virtio-net follows the behavior of an active backup bond/team
> > > > - virtio-net acts as the interface to the VM
> > > > - live migration initiates
> > > > - link goes down on SR-IOV, or SR-IOV device is removed
> > > > - failover to virtio-net as primary path
> > > > - migration continues to new host
> > > > - new host is started with virio-net as primary
> > > > - if no SR-IOV, virtio-net stays primary
> > > > - hypervisor can hot-add SR-IOV NIC, with same MAC addr as virtio
> > > > - virtio-net notices new NIC and starts over at enslave step above
> > > >
> > > > Future ideas (brainstorming):
> > > > - Optimize Fast east-west by having special rules to direct east-west
> > > > traffic through virtio-net traffic path
> > > >
> > > > Thanks for reading!
> > > > Jesse
> > >
> > > Cc netdev.
> > >
> > > Interesting, and this method is actually used by netvsc now:
> > >
> > > commit 0c195567a8f6e82ea5535cd9f1d54a1626dd233e
> > > Author: stephen hemminger <stephen@networkplumber.org>
> > > Date: Tue Aug 1 19:58:53 2017 -0700
> > >
> > > netvsc: transparent VF management
> > >
> > > This patch implements transparent fail over from synthetic NIC to
> > > SR-IOV virtual function NIC in Hyper-V environment. It is a better
> > > alternative to using bonding as is done now. Instead, the receive and
> > > transmit fail over is done internally inside the driver.
> > >
> > > Using bonding driver has lots of issues because it depends on the
> > > script being run early enough in the boot process and with sufficient
> > > information to make the association. This patch moves all that
> > > functionality into the kernel.
> > >
> > > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> > > Signed-off-by: David S. Miller <davem@davemloft.net>
> > >
> > > If my understanding is correct there's no need to for any extension of
> > > virtio spec. If this is true, maybe you can start to prepare the patch?
> >
> > IMHO this is as close to policy in the kernel as one can get. User
> > land has all the information it needs to instantiate that bond/team
> > automatically.
>
> It does have this info (MAC addresses match) but where's the policy
> here? IMHO the policy has been set by the hypervisor already.
> From hypervisor POV adding passthrough is a commitment not to migrate
> until guest stops using the passthrough device.
>
> Within the guest, the bond is required for purely functional reasons - just to
> maintain a link up since we know SRIOV will will go away. Maintaining an
> uninterrupted connection is not a policy - it's what networking is
> about.
>
> > In fact I'm trying to discuss this with NetworkManager
> > folks and Red Hat right now:
> >
> > https://mail.gnome.org/archives/networkmanager-list/2017-November/msg00038.html
>
> I thought we should do it too, for a while.
>
> But now, I think that the real issue is this: kernel exposes what looks
> like two network devices to userspace, but in fact it is just one
> backend device, just exposed by hypervisor in a weird way for
> compatibility reasons.
>
> For example you will not get a better reliability or throughput by using
> both of them - the only bonding mode that makes sense is fail over.
Yes, I'm talking about fail over.
> As another example, if the underlying physical device lost its link, trying
> to use virtio won't help - it's only useful when the passthrough device
> is gone for good. As another example, there is no point in not
> configuring a bond. As a last example, depending on how the backend is
> configured, virtio might not even work when the pass-through device is
> active.
>
> So from that point of view, showing two network devices to userspace is
> a bug that we are asking userspace to work around.
I'm confused by what you're saying here. IIRC the question is whether
we expose 2 netdevs or 3. There will always be a virtio netdev and a
VF netdev. I assume you're not suggesting hiding the VF netdev. So
the question is do we expose a VF netdev and a combo virtio netdev
which is also a bond or do we expose a VF netdev a virtio netdev, and a
active/passive bond/team which is a well understood and architecturally
correct construct.
> > Can we flip the argument and ask why is the kernel supposed to be
> > responsible for this?
>
> Because if we show a single device to userspace the number of
> misconfigured guests will go down, and we won't lose any useful
> flexibility.
Again, single device?
> > It's not like we run DHCP out of the kernel
> > on new interfaces...
>
> Because one can set up a static IP, IPv6 doesn't always need DHCP, etc.
But we don't handle LACP, etc.
Look, as much as I don't like this, I'm not going to argue about this to
death. I just find it very dishonest to claim kernel *has to* do it,
when no one seem to have made any honest attempts to solve this in user
space for the last 10 years :/
^ permalink raw reply
* Re: [PATCH net-next 3/3] rds: tcp: atomically purge entries from rds_tcp_conn_list during netns delete
From: Santosh Shilimkar @ 2017-11-30 20:38 UTC (permalink / raw)
To: Sowmini Varadhan, netdev; +Cc: davem, rds-devel
In-Reply-To: <7935e3668947d91c69a00a3b3db7489d5d2a4325.1511195030.git.sowmini.varadhan@oracle.com>
On 11/30/2017 11:11 AM, Sowmini Varadhan wrote:
> The rds_tcp_kill_sock() function parses the rds_tcp_conn_list
> to find the rds_connection entries marked for deletion as part
> of the netns deletion under the protection of the rds_tcp_conn_lock.
> Since the rds_tcp_conn_list tracks rds_tcp_connections (which
> have a 1:1 mapping with rds_conn_path), multiple tc entries in
> the rds_tcp_conn_list will map to a single rds_connection, and will
> be deleted as part of the rds_conn_destroy() operation that is
> done outside the rds_tcp_conn_lock.
>
> The rds_tcp_conn_list traversal done under the protection of
> rds_tcp_conn_lock should not leave any doomed tc entries in
> the list after the rds_tcp_conn_lock is released, else another
> concurrently executiong netns delete (for a differnt netns) thread
> may trip on these entries.
>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
^ permalink raw reply
* Re: [PATCH net-next 2/3] rds: tcp: correctly sequence cleanup on netns deletion.
From: Santosh Shilimkar @ 2017-11-30 20:28 UTC (permalink / raw)
To: Sowmini Varadhan, netdev; +Cc: davem, rds-devel
In-Reply-To: <4c8332354d9423207740a814ea75706f59ca7e72.1511195030.git.sowmini.varadhan@oracle.com>
On 11/30/2017 11:11 AM, Sowmini Varadhan wrote:
> Commit 8edc3affc077 ("rds: tcp: Take explicit refcounts on struct net")
> introduces a regression in rds-tcp netns cleanup. The cleanup_net(),
> (and thus rds_tcp_dev_event notification) is only called from put_net()
> when all netns refcounts go to 0, but this cannot happen if the
> rds_connection itself is holding a c_net ref that it expects to
> release in rds_tcp_kill_sock.
>
> Instead, the rds_tcp_kill_sock callback should make sure to
> tear down state carefully, ensuring that the socket teardown
> is only done after all data-structures and workqs that depend
> on it are quiesced.
>
> The original motivation for commit 8edc3affc077 ("rds: tcp: Take explicit
> refcounts on struct net") was to resolve a race condition reported by
> syzkaller where workqs for tx/rx/connect were triggered after the
> namespace was deleted. Those worker threads should have been
> cancelled/flushed before socket tear-down and indeed,
> rds_conn_path_destroy() does try to sequence this by doing
> /* cancel cp_send_w */
> /* cancel cp_recv_w */
> /* flush cp_down_w */
> /* free data structures */
> Here the "flush cp_down_w" will trigger rds_conn_shutdown and thus
> invoke rds_tcp_conn_path_shutdown() to close the tcp socket, so that
> we ought to have satisfied the requirement that "socket-close is
> done after all other dependent state is quiesced". However,
> rds_conn_shutdown has a bug in that it *always* triggers the reconnect
> workq (and if connection is successful, we always restart tx/rx
> workqs so with the right timing, we risk the race conditions reported
> by syzkaller).
>
> Netns deletion is like module teardown- no need to restart a
> reconnect in this case. We can use the c_destroy_in_prog bit
> to avoid restarting the reconnect.
>
> Fixes: 8edc3affc077 ("rds: tcp: Take explicit refcounts on struct net")
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> net/rds/connection.c | 3 ++-
> net/rds/rds.h | 6 +++---
> net/rds/tcp.c | 4 ++--
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index 7ee2d5d..9efc82c 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -366,6 +366,8 @@ void rds_conn_shutdown(struct rds_conn_path *cp)
> * to the conn hash, so we never trigger a reconnect on this
> * conn - the reconnect is always triggered by the active peer. */
> cancel_delayed_work_sync(&cp->cp_conn_w);
> + if (conn->c_destroy_in_prog)
> + return;
Not related to this patch but it will be more safe to use
cp_flags or if needed add flag and conn level for bundle
and use bit wise to avoid possible races to set c_destroy_in_prog.
Something similar to RDS_DESTROY_PENDING etc.
The patch itself looks good to me in terms of netns ref counting.
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
^ permalink raw reply
* Re: [PATCH net-next 1/3] rds: tcp: remove redundant function rds_tcp_conn_paths_destroy()
From: Santosh Shilimkar @ 2017-11-30 20:20 UTC (permalink / raw)
To: Sowmini Varadhan, netdev; +Cc: davem, rds-devel
In-Reply-To: <01449f728c031e98e370ac78e6a7d714aec8cfca.1511195030.git.sowmini.varadhan@oracle.com>
On 11/30/2017 11:11 AM, Sowmini Varadhan wrote:
> A side-effect of Commit c14b0366813a ("rds: tcp: set linger to 1
> when unloading a rds-tcp") is that we always send a RST on the tcp
> connection for rds_conn_destroy(), so rds_tcp_conn_paths_destroy()
> is not needed any more and is removed in this patch.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
Looks good.
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
^ 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