* [PATCH v2 net-next 1/3] net: dsa: sja1105: Don't check state->link in phylink_mac_config
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
To: linux, f.fainelli, vivien.didelot, andrew, davem
Cc: netdev, Vladimir Oltean, Russell King
In-Reply-To: <20190627214637.22366-1-olteanv@gmail.com>
It has been pointed out that PHYLINK can call mac_config only to update
the phy_interface_type and without knowing what the AN results are.
Experimentally, when this was observed to happen, state->link was also
unset, and therefore was used as a proxy to ignore this call. However it
is also suggested that state->link is undefined for this callback and
should not be relied upon.
So let the previously-dead codepath for SPEED_UNKNOWN be called, and
update the comment to make sure the MAC's behavior is sane.
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index caebf76eaa3e..da1736093b06 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -715,7 +715,13 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
switch (speed_mbps) {
case SPEED_UNKNOWN:
- /* No speed update requested */
+ /* PHYLINK called sja1105_mac_config() to inform us about
+ * the state->interface, but AN has not completed and the
+ * speed is not yet valid. UM10944.pdf says that setting
+ * SJA1105_SPEED_AUTO at runtime disables the port, so that is
+ * ok for power consumption in case AN will never complete -
+ * otherwise PHYLINK should come back with a new update.
+ */
speed = SJA1105_SPEED_AUTO;
break;
case SPEED_10:
@@ -766,9 +772,6 @@ static void sja1105_mac_config(struct dsa_switch *ds, int port,
{
struct sja1105_private *priv = ds->priv;
- if (!state->link)
- return;
-
sja1105_adjust_port_config(priv, port, state->speed);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 2/3] net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK reports
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
To: linux, f.fainelli, vivien.didelot, andrew, davem
Cc: netdev, Vladimir Oltean, Russell King
In-Reply-To: <20190627214637.22366-1-olteanv@gmail.com>
PHYLINK being designed with PHYs in mind that can change MII protocol,
for correct operation it is necessary to ensure that the PHY interface
mode stays the same (otherwise clear the supported bit mask, as
required).
Because this is just a hypothetical situation for now, we don't bother
to check whether we could actually support the new PHY interface mode.
Actually we could modify the xMII table, reset the switch and send an
updated static configuration, but adding that would just be dead code.
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 44 ++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index da1736093b06..b366b8e100f8 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -766,12 +766,46 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
return sja1105_clocking_setup_port(priv, port);
}
+/* The SJA1105 MAC programming model is through the static config (the xMII
+ * Mode table cannot be dynamically reconfigured), and we have to program
+ * that early (earlier than PHYLINK calls us, anyway).
+ * So just error out in case the connected PHY attempts to change the initial
+ * system interface MII protocol from what is defined in the DT, at least for
+ * now.
+ */
+static bool sja1105_phy_mode_mismatch(struct sja1105_private *priv, int port,
+ phy_interface_t interface)
+{
+ struct sja1105_xmii_params_entry *mii;
+ sja1105_phy_interface_t phy_mode;
+
+ mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
+ phy_mode = mii->xmii_mode[port];
+
+ switch (interface) {
+ case PHY_INTERFACE_MODE_MII:
+ return (phy_mode != XMII_MODE_MII);
+ case PHY_INTERFACE_MODE_RMII:
+ return (phy_mode != XMII_MODE_RMII);
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ return (phy_mode != XMII_MODE_RGMII);
+ default:
+ return true;
+ }
+}
+
static void sja1105_mac_config(struct dsa_switch *ds, int port,
unsigned int link_an_mode,
const struct phylink_link_state *state)
{
struct sja1105_private *priv = ds->priv;
+ if (sja1105_phy_mode_mismatch(priv, port, state->interface))
+ return;
+
sja1105_adjust_port_config(priv, port, state->speed);
}
@@ -804,6 +838,16 @@ static void sja1105_phylink_validate(struct dsa_switch *ds, int port,
mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
+ /* include/linux/phylink.h says:
+ * When @state->interface is %PHY_INTERFACE_MODE_NA, phylink
+ * expects the MAC driver to return all supported link modes.
+ */
+ if (state->interface != PHY_INTERFACE_MODE_NA &&
+ sja1105_phy_mode_mismatch(priv, port, state->interface)) {
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ return;
+ }
+
/* The MAC does not support pause frames, and also doesn't
* support half-duplex traffic modes.
*/
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 3/3] net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
To: linux, f.fainelli, vivien.didelot, andrew, davem
Cc: netdev, Vladimir Oltean, Russell King
In-Reply-To: <20190627214637.22366-1-olteanv@gmail.com>
We need a better way to signal this, perhaps in phylink_validate, but
for now just print this error message as guidance for other people
looking at this driver's code while trying to rework PHYLINK.
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index b366b8e100f8..32bf3a7cc3b6 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -806,6 +806,11 @@ static void sja1105_mac_config(struct dsa_switch *ds, int port,
if (sja1105_phy_mode_mismatch(priv, port, state->interface))
return;
+ if (link_an_mode == MLO_AN_INBAND) {
+ dev_err(ds->dev, "In-band AN not supported!\n");
+ return;
+ }
+
sja1105_adjust_port_config(priv, port, state->speed);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Daniel Borkmann @ 2019-06-27 21:55 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <156125626136.5209.14349225282974871197.stgit@alrua-x1>
On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> The bpf_redirect_map() helper used by XDP programs doesn't return any
> indication of whether it can successfully redirect to the map index it was
> given. Instead, BPF programs have to track this themselves, leading to
> programs using duplicate maps to track which entries are populated in the
> devmap.
>
> This patch fixes this by moving the map lookup into the bpf_redirect_map()
> helper, which makes it possible to return failure to the eBPF program. The
> lower bits of the flags argument is used as the return code, which means
> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>
> With this, a BPF program can check the return code from the helper call and
> react by, for instance, substituting a different redirect. This works for
> any type of map used for redirect.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Overall series looks good to me. Just very small things inline here & in the
other two patches:
[...]
> @@ -3750,9 +3742,16 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
> {
> struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
>
> - if (unlikely(flags))
> + /* Lower bits of the flags are used as return code on lookup failure */
> + if (unlikely(flags > XDP_TX))
> return XDP_ABORTED;
>
> + ri->item = __xdp_map_lookup_elem(map, ifindex);
> + if (unlikely(!ri->item)) {
> + WRITE_ONCE(ri->map, NULL);
This WRITE_ONCE() is not needed. We never set it before at this point.
> + return flags;
> + }
> +
> ri->ifindex = ifindex;
> ri->flags = flags;
> WRITE_ONCE(ri->map, map);
>
^ permalink raw reply
* [PATCH 0/5] net: dsa: microchip: Further regmap cleanups
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
This patchset cleans up KSZ9477 switch driver by replacing various
ad-hoc polling implementations and register RMW with regmap functions.
Each polling function is replaced separately to make it easier to review
and possibly bisect, but maybe the patches can be squashed.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
Marek Vasut (5):
net: dsa: microchip: Replace ad-hoc polling with regmap
net: dsa: microchip: Replace ksz9477_wait_vlan_ctrl_ready polling with
regmap
net: dsa: microchip: Replace ksz9477_wait_alu_ready polling with
regmap
net: dsa: microchip: Replace ksz9477_wait_alu_sta_ready polling with
regmap
net: dsa: microchip: Replace bit RMW with regmap
drivers/net/dsa/microchip/ksz9477.c | 128 +++++++++----------------
drivers/net/dsa/microchip/ksz_common.h | 14 ---
2 files changed, 47 insertions(+), 95 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 1/5] net: dsa: microchip: Replace ad-hoc polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>
Regmap provides polling function to poll for bits in a register,
use in instead of reimplementing it.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
drivers/net/dsa/microchip/ksz9477.c | 14 +++++---------
drivers/net/dsa/microchip/ksz_common.h | 14 --------------
2 files changed, 5 insertions(+), 23 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 8f13dcc05a10..ece25f38e02a 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -263,12 +263,8 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
u64 *cnt)
{
- struct ksz_poll_ctx ctx = {
- .dev = dev,
- .port = port,
- .offset = REG_PORT_MIB_CTRL_STAT__4,
- };
struct ksz_port *p = &dev->ports[port];
+ unsigned int val;
u32 data;
int ret;
@@ -278,11 +274,11 @@ static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
data |= (addr << MIB_COUNTER_INDEX_S);
ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
- ret = readx_poll_timeout(ksz_pread32_poll, &ctx, data,
- !(data & MIB_COUNTER_READ), 10, 1000);
-
+ ret = regmap_read_poll_timeout(dev->regmap[2],
+ PORT_CTRL_ADDR(port, REG_PORT_MIB_CTRL_STAT__4),
+ val, !(val & MIB_COUNTER_READ), 10, 1000);
/* failed to read MIB. get out of loop */
- if (ret < 0) {
+ if (ret) {
dev_dbg(dev->dev, "Failed to get MIB\n");
return;
}
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 745318424f71..ee7096d8af07 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -119,20 +119,6 @@ static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
ksz_write32(dev, dev->dev_ops->get_port_addr(port, offset), data);
}
-struct ksz_poll_ctx {
- struct ksz_device *dev;
- int port;
- int offset;
-};
-
-static inline u32 ksz_pread32_poll(struct ksz_poll_ctx *ctx)
-{
- u32 data;
-
- ksz_pread32(ctx->dev, ctx->port, ctx->offset, &data);
- return data;
-}
-
/* Regmap tables generation */
#define KSZ_SPI_OP_RD 3
#define KSZ_SPI_OP_WR 2
--
2.20.1
^ permalink raw reply related
* [PATCH 2/5] net: dsa: microchip: Replace ksz9477_wait_vlan_ctrl_ready polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>
Regmap provides polling function to poll for bits in a register. This
function is another reimplementation of polling for bit being clear in
a register. Replace this with regmap polling function. Moreover, inline
the function parameters, as the function is never called with any other
parameter values than this one.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
drivers/net/dsa/microchip/ksz9477.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index ece25f38e02a..0aab00bf23b9 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -89,22 +89,12 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset,
bits, set ? bits : 0);
}
-static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton,
- int timeout)
+static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev)
{
- u8 data;
-
- do {
- ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
- if (!(data & waiton))
- break;
- usleep_range(1, 10);
- } while (timeout-- > 0);
-
- if (timeout <= 0)
- return -ETIMEDOUT;
+ unsigned int val;
- return 0;
+ return regmap_read_poll_timeout(dev->regmap[0], REG_SW_VLAN_CTRL,
+ val, !(val & VLAN_START), 10, 1000);
}
static int ksz9477_get_vlan_table(struct ksz_device *dev, u16 vid,
@@ -118,8 +108,8 @@ static int ksz9477_get_vlan_table(struct ksz_device *dev, u16 vid,
ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
/* wait to be cleared */
- ret = ksz9477_wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
- if (ret < 0) {
+ ret = ksz9477_wait_vlan_ctrl_ready(dev);
+ if (ret) {
dev_dbg(dev->dev, "Failed to read vlan table\n");
goto exit;
}
@@ -151,8 +141,8 @@ static int ksz9477_set_vlan_table(struct ksz_device *dev, u16 vid,
ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
/* wait to be cleared */
- ret = ksz9477_wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
- if (ret < 0) {
+ ret = ksz9477_wait_vlan_ctrl_ready(dev);
+ if (ret) {
dev_dbg(dev->dev, "Failed to write vlan table\n");
goto exit;
}
--
2.20.1
^ permalink raw reply related
* [PATCH 4/5] net: dsa: microchip: Replace ksz9477_wait_alu_sta_ready polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>
Regmap provides polling function to poll for bits in a register. This
function is another reimplementation of polling for bit being clear in
a register. Replace this with regmap polling function. Moreover, inline
the function parameters, as the function is never called with any other
parameter values than this one.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
drivers/net/dsa/microchip/ksz9477.c | 32 +++++++++++------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index e5b2f3e45db6..bfc44799854c 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -184,22 +184,14 @@ static int ksz9477_wait_alu_ready(struct ksz_device *dev)
val, !(val & ALU_START), 10, 1000);
}
-static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev, u32 waiton,
- int timeout)
+static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev)
{
- u32 data;
-
- do {
- ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
- if (!(data & waiton))
- break;
- usleep_range(1, 10);
- } while (timeout-- > 0);
-
- if (timeout <= 0)
- return -ETIMEDOUT;
+ unsigned int val;
- return 0;
+ return regmap_read_poll_timeout(dev->regmap[2],
+ REG_SW_ALU_STAT_CTRL__4,
+ val, !(val & ALU_STAT_START),
+ 10, 1000);
}
static int ksz9477_reset_switch(struct ksz_device *dev)
@@ -821,7 +813,7 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
/* wait to be finished */
- if (ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0) {
+ if (ksz9477_wait_alu_sta_ready(dev)) {
dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
goto exit;
}
@@ -862,7 +854,7 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
/* wait to be finished */
- if (ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0)
+ if (ksz9477_wait_alu_sta_ready(dev))
dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
exit:
@@ -892,8 +884,8 @@ static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
/* wait to be finished */
- ret = ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
- if (ret < 0) {
+ ret = ksz9477_wait_alu_sta_ready(dev);
+ if (ret) {
dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
goto exit;
}
@@ -934,8 +926,8 @@ static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
/* wait to be finished */
- ret = ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
- if (ret < 0)
+ ret = ksz9477_wait_alu_sta_ready(dev);
+ if (ret)
dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
exit:
--
2.20.1
^ permalink raw reply related
* [PATCH 5/5] net: dsa: microchip: Replace bit RMW with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>
Regmap provides read-modify-write function to update bitfields in
registers. Replace ad-hoc read-modify-write with regmap_update_bits()
where applicable.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
drivers/net/dsa/microchip/ksz9477.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index bfc44799854c..a8c97f7a79b7 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -197,16 +197,14 @@ static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev)
static int ksz9477_reset_switch(struct ksz_device *dev)
{
u8 data8;
- u16 data16;
u32 data32;
/* reset switch */
ksz_cfg(dev, REG_SW_OPERATION, SW_RESET, true);
/* turn off SPI DO Edge select */
- ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
- data8 &= ~SPI_AUTO_EDGE_DETECTION;
- ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
+ regmap_update_bits(dev->regmap[0], REG_SW_GLOBAL_SERIAL_CTRL_0,
+ SPI_AUTO_EDGE_DETECTION, 0);
/* default configuration */
ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
@@ -220,10 +218,10 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
/* set broadcast storm protection 10% rate */
- ksz_read16(dev, REG_SW_MAC_CTRL_2, &data16);
- data16 &= ~BROADCAST_STORM_RATE;
- data16 |= (BROADCAST_STORM_VALUE * BROADCAST_STORM_PROT_RATE) / 100;
- ksz_write16(dev, REG_SW_MAC_CTRL_2, data16);
+ regmap_update_bits(dev->regmap[1], REG_SW_MAC_CTRL_2,
+ BROADCAST_STORM_RATE,
+ (BROADCAST_STORM_VALUE *
+ BROADCAST_STORM_PROT_RATE) / 100);
if (dev->synclko_125)
ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1,
@@ -485,10 +483,10 @@ static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
{
u8 data;
- ksz_read8(dev, REG_SW_LUE_CTRL_2, &data);
- data &= ~(SW_FLUSH_OPTION_M << SW_FLUSH_OPTION_S);
- data |= (SW_FLUSH_OPTION_DYN_MAC << SW_FLUSH_OPTION_S);
- ksz_write8(dev, REG_SW_LUE_CTRL_2, data);
+ regmap_update_bits(dev->regmap[0], REG_SW_LUE_CTRL_2,
+ SW_FLUSH_OPTION_M << SW_FLUSH_OPTION_S,
+ SW_FLUSH_OPTION_DYN_MAC << SW_FLUSH_OPTION_S);
+
if (port < dev->mib_port_cnt) {
/* flush individual port */
ksz_pread8(dev, port, P_STP_CTRL, &data);
--
2.20.1
^ permalink raw reply related
* [PATCH 3/5] net: dsa: microchip: Replace ksz9477_wait_alu_ready polling with regmap
From: Marek Vasut @ 2019-06-27 21:55 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190627215556.23768-1-marex@denx.de>
Regmap provides polling function to poll for bits in a register. This
function is another reimplementation of polling for bit being clear in
a register. Replace this with regmap polling function. Moreover, inline
the function parameters, as the function is never called with any other
parameter values than this one.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
drivers/net/dsa/microchip/ksz9477.c | 34 ++++++++++-------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 0aab00bf23b9..e5b2f3e45db6 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -176,22 +176,12 @@ static void ksz9477_write_table(struct ksz_device *dev, u32 *table)
ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]);
}
-static int ksz9477_wait_alu_ready(struct ksz_device *dev, u32 waiton,
- int timeout)
+static int ksz9477_wait_alu_ready(struct ksz_device *dev)
{
- u32 data;
-
- do {
- ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
- if (!(data & waiton))
- break;
- usleep_range(1, 10);
- } while (timeout-- > 0);
-
- if (timeout <= 0)
- return -ETIMEDOUT;
+ unsigned int val;
- return 0;
+ return regmap_read_poll_timeout(dev->regmap[2], REG_SW_ALU_CTRL__4,
+ val, !(val & ALU_START), 10, 1000);
}
static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev, u32 waiton,
@@ -633,8 +623,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
/* wait to be finished */
- ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
- if (ret < 0) {
+ ret = ksz9477_wait_alu_ready(dev);
+ if (ret) {
dev_dbg(dev->dev, "Failed to read ALU\n");
goto exit;
}
@@ -657,8 +647,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
/* wait to be finished */
- ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
- if (ret < 0)
+ ret = ksz9477_wait_alu_ready(dev);
+ if (ret)
dev_dbg(dev->dev, "Failed to write ALU\n");
exit:
@@ -690,8 +680,8 @@ static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
/* wait to be finished */
- ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
- if (ret < 0) {
+ ret = ksz9477_wait_alu_ready(dev);
+ if (ret) {
dev_dbg(dev->dev, "Failed to read ALU\n");
goto exit;
}
@@ -724,8 +714,8 @@ static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
/* wait to be finished */
- ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
- if (ret < 0)
+ ret = ksz9477_wait_alu_ready(dev);
+ if (ret)
dev_dbg(dev->dev, "Failed to write ALU\n");
exit:
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Ivan Khoronzhuk @ 2019-06-27 22:02 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: davem, grygorii.strashko, saeedm, leon, ast, linux-kernel,
linux-omap, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend
In-Reply-To: <20190627214317.237e5926@carbon>
Hi Jesper, thanks you remember about it.
>
>I don't think that "create" and "free" routines paring looks "more
>correct" together.
>
>Maybe we can scale back your solution(?), via creating a page_pool_get()
>and page_pool_put() API that can be used by your driver, to keep the
>page_pool object after a xdp_rxq_info_unreg() call. Then you can use
>it for two xdp_rxq_info structs, and call page_pool_put() after you
>have unregistered both.
>
>The API would basically be:
>
>diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>index b366f59885c1..691ddacfb5a6 100644
>--- a/net/core/page_pool.c
>+++ b/net/core/page_pool.c
>@@ -357,6 +357,10 @@ static void __warn_in_flight(struct page_pool *pool)
> void __page_pool_free(struct page_pool *pool)
> {
> WARN(pool->alloc.count, "API usage violation");
>+
>+ if (atomic_read(&pool->user_cnt) != 0)
>+ return;
>+
> WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>
> /* Can happen due to forced shutdown */
>@@ -372,6 +376,19 @@ void __page_pool_free(struct page_pool *pool)
> }
> EXPORT_SYMBOL(__page_pool_free);
>
>+void page_pool_put(struct page_pool *pool)
>+{
>+ if (!atomic_dec_and_test(&pool->user_cnt))
>+ __page_pool_free(pool);
>+}
>+EXPORT_SYMBOL(page_pool_put);
>+
>+void page_pool_get(struct page_pool *pool)
>+{
>+ atomic_inc(&pool->user_cnt);
>+}
>+EXPORT_SYMBOL(page_pool_get);
>+
I have another solution that doesn't touch page pool and adds modifications
to xdp allocator. As for me it looks better and work wider, I don't need to
think about this in the driver also.
It's supposed allocator works as before, no any changes to mlx5 and
page_pool API and its usage and seems like fits your requirements.
It still supposes that allocator runs under same napi softirq but allows
to reuse allocator.
I have not verified yet, but looks like:
diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
index 6a8cba6ea79a..995b21da2f27 100644
--- a/include/net/xdp_priv.h
+++ b/include/net/xdp_priv.h
@@ -18,6 +18,7 @@ struct xdp_mem_allocator {
struct rcu_head rcu;
struct delayed_work defer_wq;
unsigned long defer_warn;
+ unsigned long refcnt;
};
#endif /* __LINUX_NET_XDP_PRIV_H__ */
diff --git a/net/core/xdp.c b/net/core/xdp.c
index f98ab6b98674..6239483e3793 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -98,6 +98,12 @@ static bool __mem_id_disconnect(int id, bool force)
WARN(1, "Request remove non-existing id(%d), driver bug?", id);
return true;
}
+
+ if (--xa->refcnt) {
+ mutex_unlock(&mem_id_lock);
+ return true;
+ }
+
xa->disconnect_cnt++;
/* Detects in-flight packet-pages for page_pool */
@@ -312,6 +318,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
return true;
}
+static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
+{
+ struct xdp_mem_allocator *xae, *xa == NULL;
+ struct rhashtable_iter iter;
+
+ mutex_lock(&mem_id_lock);
+ rhashtable_walk_enter(mem_id_ht, &iter);
+ do {
+ rhashtable_walk_start(&iter);
+
+ while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
+ if (xae->allocator == allocator) {
+ xae->refcnt++;
+ xa = xae;
+ break;
+ }
+ }
+
+ rhashtable_walk_stop(&iter);
+
+ } while (xae == ERR_PTR(-EAGAIN));
+ rhashtable_walk_exit(&iter);
+ mutex_unlock(&mem_id_lock);
+
+ return xa;
+}
+
int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
enum xdp_mem_type type, void *allocator)
{
@@ -347,6 +380,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
}
}
+ if (xdp_allocator_get(allocator))
+ return 0;
+
xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
if (!xdp_alloc)
return -ENOMEM;
@@ -360,6 +396,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
xdp_rxq->mem.id = id;
xdp_alloc->mem = xdp_rxq->mem;
xdp_alloc->allocator = allocator;
+ xdp_alloc->refcnt = 1;
/* Insert allocator into ID lookup table */
ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply related
* Re: WARNING in mark_lock
From: syzbot @ 2019-06-27 22:03 UTC (permalink / raw)
To: ast, bpf, daniel, dvyukov, ebiggers, john.fastabend, linux-kernel,
netdev, peterz, syzkaller-bugs, tglx
In-Reply-To: <0000000000005aedf1058c1bf7e8@google.com>
syzbot has bisected this bug to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1436e7e9a00000
start commit: dc636f5d Add linux-next specific files for 20190620
git tree: linux-next
final crash: https://syzkaller.appspot.com/x/report.txt?x=1636e7e9a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1236e7e9a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=99c104b0092a557b
dashboard link: https://syzkaller.appspot.com/bug?extid=a861f52659ae2596492b
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=110b24f6a00000
Reported-by: syzbot+a861f52659ae2596492b@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH bpf v5 1/2] xdp: hold device for umem regardless of zero-copy mode
From: Jonathan Lemon @ 2019-06-27 22:04 UTC (permalink / raw)
To: Ilya Maximets
Cc: netdev, linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jakub Kicinski,
Daniel Borkmann
In-Reply-To: <20190627101529.11234-2-i.maximets@samsung.com>
On 27 Jun 2019, at 3:15, Ilya Maximets wrote:
> Device pointer stored in umem regardless of zero-copy mode,
> so we heed to hold the device in all cases.
>
> Fixes: c9b47cc1fabc ("xsk: fix bug when trying to use both copy and
> zero-copy on one queue id")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf v5 2/2] xdp: fix hang while unregistering device bound to xdp socket
From: Jonathan Lemon @ 2019-06-27 22:04 UTC (permalink / raw)
To: Ilya Maximets
Cc: netdev, linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jakub Kicinski,
Daniel Borkmann
In-Reply-To: <20190627101529.11234-3-i.maximets@samsung.com>
On 27 Jun 2019, at 3:15, Ilya Maximets wrote:
> Device that bound to XDP socket will not have zero refcount until the
> userspace application will not close it. This leads to hang inside
> 'netdev_wait_allrefs()' if device unregistering requested:
>
> # ip link del p1
> < hang on recvmsg on netlink socket >
>
> # ps -x | grep ip
> 5126 pts/0 D+ 0:00 ip link del p1
>
> # journalctl -b
>
> Jun 05 07:19:16 kernel:
> unregister_netdevice: waiting for p1 to become free. Usage count = 1
>
> Jun 05 07:19:27 kernel:
> unregister_netdevice: waiting for p1 to become free. Usage count = 1
> ...
>
> Fix that by implementing NETDEV_UNREGISTER event notification handler
> to properly clean up all the resources and unref device.
>
> This should also allow socket killing via ss(8) utility.
>
> Fixes: 965a99098443 ("xsk: add support for bind for Rx")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
> include/net/xdp_sock.h | 5 +++
> net/xdp/xdp_umem.c | 10 ++---
> net/xdp/xdp_umem.h | 1 +
> net/xdp/xsk.c | 87
> ++++++++++++++++++++++++++++++++++++------
> 4 files changed, 87 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index d074b6d60f8a..82d153a637c7 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -61,6 +61,11 @@ struct xdp_sock {
> struct xsk_queue *tx ____cacheline_aligned_in_smp;
> struct list_head list;
> bool zc;
> + enum {
> + XSK_UNINITIALIZED = 0,
> + XSK_BINDED,
> + XSK_UNBINDED,
> + } state;
I'd prefer that these were named better, perhaps:
XSK_READY,
XSK_BOUND,
XSK_UNBOUND,
Other than that:
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
--
Jonathan
> /* Protects multiple processes in the control path */
> struct mutex mutex;
> /* Mutual exclusion of NAPI TX thread and sendmsg error paths
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 267b82a4cbcf..20c91f02d3d8 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -140,11 +140,13 @@ int xdp_umem_assign_dev(struct xdp_umem *umem,
> struct net_device *dev,
> return err;
> }
>
> -static void xdp_umem_clear_dev(struct xdp_umem *umem)
> +void xdp_umem_clear_dev(struct xdp_umem *umem)
> {
> struct netdev_bpf bpf;
> int err;
>
> + ASSERT_RTNL();
> +
> if (!umem->dev)
> return;
>
> @@ -153,17 +155,13 @@ static void xdp_umem_clear_dev(struct xdp_umem
> *umem)
> bpf.xsk.umem = NULL;
> bpf.xsk.queue_id = umem->queue_id;
>
> - rtnl_lock();
> err = umem->dev->netdev_ops->ndo_bpf(umem->dev, &bpf);
> - rtnl_unlock();
>
> if (err)
> WARN(1, "failed to disable umem!\n");
> }
>
> - rtnl_lock();
> xdp_clear_umem_at_qid(umem->dev, umem->queue_id);
> - rtnl_unlock();
>
> dev_put(umem->dev);
> umem->dev = NULL;
> @@ -195,7 +193,9 @@ static void xdp_umem_unaccount_pages(struct
> xdp_umem *umem)
>
> static void xdp_umem_release(struct xdp_umem *umem)
> {
> + rtnl_lock();
> xdp_umem_clear_dev(umem);
> + rtnl_unlock();
>
> ida_simple_remove(&umem_ida, umem->id);
>
> diff --git a/net/xdp/xdp_umem.h b/net/xdp/xdp_umem.h
> index 27603227601b..a63a9fb251f5 100644
> --- a/net/xdp/xdp_umem.h
> +++ b/net/xdp/xdp_umem.h
> @@ -10,6 +10,7 @@
>
> int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device
> *dev,
> u16 queue_id, u16 flags);
> +void xdp_umem_clear_dev(struct xdp_umem *umem);
> bool xdp_umem_validate_queues(struct xdp_umem *umem);
> void xdp_get_umem(struct xdp_umem *umem);
> void xdp_put_umem(struct xdp_umem *umem);
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index a14e8864e4fa..336723948a36 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -335,6 +335,22 @@ static int xsk_init_queue(u32 entries, struct
> xsk_queue **queue,
> return 0;
> }
>
> +static void xsk_unbind_dev(struct xdp_sock *xs)
> +{
> + struct net_device *dev = xs->dev;
> +
> + if (!dev || xs->state != XSK_BINDED)
> + return;
> +
> + xs->state = XSK_UNBINDED;
> +
> + /* Wait for driver to stop using the xdp socket. */
> + xdp_del_sk_umem(xs->umem, xs);
> + xs->dev = NULL;
> + synchronize_net();
> + dev_put(dev);
> +}
> +
> static int xsk_release(struct socket *sock)
> {
> struct sock *sk = sock->sk;
> @@ -354,15 +370,7 @@ static int xsk_release(struct socket *sock)
> sock_prot_inuse_add(net, sk->sk_prot, -1);
> local_bh_enable();
>
> - if (xs->dev) {
> - struct net_device *dev = xs->dev;
> -
> - /* Wait for driver to stop using the xdp socket. */
> - xdp_del_sk_umem(xs->umem, xs);
> - xs->dev = NULL;
> - synchronize_net();
> - dev_put(dev);
> - }
> + xsk_unbind_dev(xs);
>
> xskq_destroy(xs->rx);
> xskq_destroy(xs->tx);
> @@ -412,7 +420,7 @@ static int xsk_bind(struct socket *sock, struct
> sockaddr *addr, int addr_len)
> return -EINVAL;
>
> mutex_lock(&xs->mutex);
> - if (xs->dev) {
> + if (xs->state != XSK_UNINITIALIZED) {
> err = -EBUSY;
> goto out_release;
> }
> @@ -492,6 +500,8 @@ static int xsk_bind(struct socket *sock, struct
> sockaddr *addr, int addr_len)
> out_unlock:
> if (err)
> dev_put(dev);
> + else
> + xs->state = XSK_BINDED;
> out_release:
> mutex_unlock(&xs->mutex);
> return err;
> @@ -520,6 +530,10 @@ static int xsk_setsockopt(struct socket *sock,
> int level, int optname,
> return -EFAULT;
>
> mutex_lock(&xs->mutex);
> + if (xs->state != XSK_UNINITIALIZED) {
> + mutex_unlock(&xs->mutex);
> + return -EBUSY;
> + }
> q = (optname == XDP_TX_RING) ? &xs->tx : &xs->rx;
> err = xsk_init_queue(entries, q, false);
> mutex_unlock(&xs->mutex);
> @@ -534,7 +548,7 @@ static int xsk_setsockopt(struct socket *sock, int
> level, int optname,
> return -EFAULT;
>
> mutex_lock(&xs->mutex);
> - if (xs->umem) {
> + if (xs->state != XSK_UNINITIALIZED || xs->umem) {
> mutex_unlock(&xs->mutex);
> return -EBUSY;
> }
> @@ -561,6 +575,10 @@ static int xsk_setsockopt(struct socket *sock,
> int level, int optname,
> return -EFAULT;
>
> mutex_lock(&xs->mutex);
> + if (xs->state != XSK_UNINITIALIZED) {
> + mutex_unlock(&xs->mutex);
> + return -EBUSY;
> + }
> if (!xs->umem) {
> mutex_unlock(&xs->mutex);
> return -EINVAL;
> @@ -662,6 +680,9 @@ static int xsk_mmap(struct file *file, struct
> socket *sock,
> unsigned long pfn;
> struct page *qpg;
>
> + if (xs->state != XSK_UNINITIALIZED)
> + return -EBUSY;
> +
> if (offset == XDP_PGOFF_RX_RING) {
> q = READ_ONCE(xs->rx);
> } else if (offset == XDP_PGOFF_TX_RING) {
> @@ -693,6 +714,38 @@ static int xsk_mmap(struct file *file, struct
> socket *sock,
> size, vma->vm_page_prot);
> }
>
> +static int xsk_notifier(struct notifier_block *this,
> + unsigned long msg, void *ptr)
> +{
> + struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> + struct net *net = dev_net(dev);
> + struct sock *sk;
> +
> + switch (msg) {
> + case NETDEV_UNREGISTER:
> + mutex_lock(&net->xdp.lock);
> + sk_for_each(sk, &net->xdp.list) {
> + struct xdp_sock *xs = xdp_sk(sk);
> +
> + mutex_lock(&xs->mutex);
> + if (xs->dev == dev) {
> + sk->sk_err = ENETDOWN;
> + if (!sock_flag(sk, SOCK_DEAD))
> + sk->sk_error_report(sk);
> +
> + xsk_unbind_dev(xs);
> +
> + /* Clear device references in umem. */
> + xdp_umem_clear_dev(xs->umem);
> + }
> + mutex_unlock(&xs->mutex);
> + }
> + mutex_unlock(&net->xdp.lock);
> + break;
> + }
> + return NOTIFY_DONE;
> +}
> +
> static struct proto xsk_proto = {
> .name = "XDP",
> .owner = THIS_MODULE,
> @@ -764,6 +817,7 @@ static int xsk_create(struct net *net, struct
> socket *sock, int protocol,
> sock_set_flag(sk, SOCK_RCU_FREE);
>
> xs = xdp_sk(sk);
> + xs->state = XSK_UNINITIALIZED;
> mutex_init(&xs->mutex);
> spin_lock_init(&xs->tx_completion_lock);
>
> @@ -784,6 +838,10 @@ static const struct net_proto_family
> xsk_family_ops = {
> .owner = THIS_MODULE,
> };
>
> +static struct notifier_block xsk_netdev_notifier = {
> + .notifier_call = xsk_notifier,
> +};
> +
> static int __net_init xsk_net_init(struct net *net)
> {
> mutex_init(&net->xdp.lock);
> @@ -816,8 +874,15 @@ static int __init xsk_init(void)
> err = register_pernet_subsys(&xsk_net_ops);
> if (err)
> goto out_sk;
> +
> + err = register_netdevice_notifier(&xsk_netdev_notifier);
> + if (err)
> + goto out_pernet;
> +
> return 0;
>
> +out_pernet:
> + unregister_pernet_subsys(&xsk_net_ops);
> out_sk:
> sock_unregister(PF_XDP);
> out_proto:
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH 8/9] keys: Network namespace domain tag [ver #4]
From: David Howells @ 2019-06-27 22:07 UTC (permalink / raw)
To: Willem de Bruijn
Cc: dhowells, Eric W. Biederman, keyrings, linux-cifs, linux-nfs,
Network Development, linux-afs, dwalsh, vgoyal,
linux-security-module, linux-fsdevel, LKML
In-Reply-To: <CAF=yD-Kgdwt5=0iboxhvZz4zvNewSGow74U15mQQvO1u8VUGcw@mail.gmail.com>
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> > +#ifdef CONFIG_KEYS
> > +out_free_2:
> > + kmem_cache_free(net_cachep, net);
>
> needs
> net = NULL;
>
> to signal failure
>
> > +#endif
I've folded that into the patch and retagged, remerged and repushed.
David
^ permalink raw reply
* Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
From: Jonathan Lemon @ 2019-06-27 22:07 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Willem de Bruijn, Toke Høiland-Jørgensen,
Machulsky, Zorik, Jubran, Samih, davem, netdev, Woodhouse, David,
Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt,
Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel,
Saidi, Ali, Herrenschmidt, Benjamin, Kiyanovski, Arthur,
Daniel Borkmann, Ilias Apalodimas, Alexei Starovoitov,
Jakub Kicinski, xdp-newbies
In-Reply-To: <20190626220028.2bb12196@carbon>
On 26 Jun 2019, at 13:00, Jesper Dangaard Brouer wrote:
> On Wed, 26 Jun 2019 09:42:07 -0700 "Jonathan Lemon"
> <jonathan.lemon@gmail.com> wrote:
>
>> If all packets are collected together (like the bulk queue does), and
>> then passed to XDP, this could easily be made backwards compatible.
>> If the XDP program isn't 'multi-frag' aware, then each packet is just
>> passed in individually.
>
> My proposal#1 is XDP only access first-buffer[1], as this simplifies
> things.
>
> (AFAIK) What you are proposing is that all the buffers are passed to
> the XDP prog (in form of a iovec). I need some more details about
> your
> suggestion.
I was thinking this over yesterday - and was probably conflating packets
and buffers a bit. Suppose that for the purposes of this discussion,
we're
talking about a single packet that is split over multiple buffer areas.
Say, on RX, with header split:
buf[0] = header
buf[1] = data
For LRO (hw recv) and jumbo frames (and TSO):
buf[0] = hdr + data
buf[1] = data
buf[n] = data
GRO cases, where individual packets are reassembled by software, aren't
handled here.
> Specifically:
>
> - What is the semantic when a 3 buffer packet is input and XDP prog
> choose to return XDP_DROP for packet #2 ?
>
> - Same situation of packet #2 wants a XDP_TX or redirect?
The collection of buffers represents a single packet, so this isn't
applicable here, right?
However, just thinking about incomplete data words (aka: pullup) gives
me a headache - seems this would complicate the BPF/verifier quite a
bit.
So perhaps just restricting things to the first entry would do for now?
As far as the exact data structure used to hold the buffers, it would
be nice if it had the same layout as a bio_vec, in case someone wanted
to get clever and start transferring things over directly.
--
Jonathan
>> Of course, passing in the equivalent of a iovec requires some form of
>> loop support on the BPF side, doesn't it?
>
> The data structure used for holding these packet buffers/segments also
> needs to be discussed. I would either use an array of bio_vec[2] or
> skb_frag_t (aka skb_frag_struct). The skb_frag_t would be most
> obvious, as we already have to write this when creating an SKB, in
> skb_shared_info area. (Structs listed below signature).
>
> The problem is also that size of these structs (16 bytes) per
> buffer/segment, and we likely need to support 17 segments, as this
> need
> to be compatible with SKBs (size 272 bytes).
>
> My idea here is that we simply use the same memory area, that we have
> to
> store skb_shared_info into. As this allow us to get the SKB setup for
> free, when doing XDP_PASS or when doing SKB alloc after XDP_REDIRECT.
>
>
> [1]
> https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org#proposal1-xdp-only-access-first-buffer
>
> [2]
> https://lore.kernel.org/netdev/20190501041757.8647-1-willy@infradead.org/
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
>
>
> $ pahole -C skb_frag_struct vmlinux
> struct skb_frag_struct {
> struct {
> struct page * p; /* 0 8 */
> } page; /* 0 8 */
> __u32 page_offset; /* 8 4 */
> __u32 size; /* 12 4 */
>
> /* size: 16, cachelines: 1, members: 3 */
> /* last cacheline: 16 bytes */
> };
>
> $ pahole -C bio_vec vmlinux
> struct bio_vec {
> struct page * bv_page; /* 0 8 */
> unsigned int bv_len; /* 8 4 */
> unsigned int bv_offset; /* 12 4 */
>
> /* size: 16, cachelines: 1, members: 3 */
> /* last cacheline: 16 bytes */
> };
>
> $ pahole -C skb_shared_info vmlinux
> struct skb_shared_info {
> __u8 __unused; /* 0 1 */
> __u8 meta_len; /* 1 1 */
> __u8 nr_frags; /* 2 1 */
> __u8 tx_flags; /* 3 1 */
> short unsigned int gso_size; /* 4 2 */
> short unsigned int gso_segs; /* 6 2 */
> struct sk_buff * frag_list; /* 8 8 */
> struct skb_shared_hwtstamps hwtstamps; /* 16 8 */
> unsigned int gso_type; /* 24 4 */
> u32 tskey; /* 28 4 */
> atomic_t dataref; /* 32 0 */
>
> /* XXX 8 bytes hole, try to pack */
>
> void * destructor_arg; /* 40 8 */
> skb_frag_t frags[17]; /* 48 272 */
>
> /* size: 320, cachelines: 5, members: 13 */
> /* sum members: 312, holes: 1, sum holes: 8 */
> };
^ permalink raw reply
* [PATCH 3/6 bpf-next] Always check the recycle stack when using the umem fq.
From: Jonathan Lemon @ 2019-06-27 22:08 UTC (permalink / raw)
To: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer; +Cc: kernel-team
In-Reply-To: <20190627220836.2572684-1-jonathan.lemon@gmail.com>
The specific _rq variants are deprecated, and will be removed next.
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
include/net/xdp_sock.h | 22 +++-------------------
net/xdp/xsk.c | 22 +++++++++++++++++++---
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 7df7b417ac53..55f5f27ef22a 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -99,33 +99,17 @@ static inline dma_addr_t xdp_umem_get_dma(struct xdp_umem *umem, u64 addr)
/* Reuse-queue aware version of FILL queue helpers */
static inline bool xsk_umem_has_addrs_rq(struct xdp_umem *umem, u32 cnt)
{
- struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
-
- if (rq->length >= cnt)
- return true;
-
- return xsk_umem_has_addrs(umem, cnt - rq->length);
+ return xsk_umem_has_addrs(umem, cnt);
}
static inline u64 *xsk_umem_peek_addr_rq(struct xdp_umem *umem, u64 *addr)
{
- struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
-
- if (!rq->length)
- return xsk_umem_peek_addr(umem, addr);
-
- *addr = rq->handles[rq->length - 1] & umem->chunk_mask;
- return addr;
+ return xsk_umem_peek_addr(umem, addr);
}
static inline void xsk_umem_discard_addr_rq(struct xdp_umem *umem)
{
- struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
-
- if (!rq->length)
- xsk_umem_discard_addr(umem);
- else
- rq->length--;
+ return xsk_umem_discard_addr(umem);
}
static inline void xsk_umem_recycle_addr(struct xdp_umem *umem, u64 addr)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 74417a851ed5..fc33070b1821 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -39,19 +39,35 @@ bool xsk_is_setup_for_bpf_map(struct xdp_sock *xs)
bool xsk_umem_has_addrs(struct xdp_umem *umem, u32 cnt)
{
- return xskq_has_addrs(umem->fq, cnt);
+ struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
+
+ if (rq->length >= cnt)
+ return true;
+
+ return xskq_has_addrs(umem->fq, cnt - rq->length);
}
EXPORT_SYMBOL(xsk_umem_has_addrs);
u64 *xsk_umem_peek_addr(struct xdp_umem *umem, u64 *addr)
{
- return xskq_peek_addr(umem->fq, addr);
+ struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
+
+ if (!rq->length)
+ return xskq_peek_addr(umem->fq, addr);
+
+ *addr = rq->handles[rq->length - 1] & umem->chunk_mask;
+ return addr;
}
EXPORT_SYMBOL(xsk_umem_peek_addr);
void xsk_umem_discard_addr(struct xdp_umem *umem)
{
- xskq_discard_addr(umem->fq);
+ struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
+
+ if (!rq->length)
+ xskq_discard_addr(umem->fq);
+ else
+ rq->length--;
}
EXPORT_SYMBOL(xsk_umem_discard_addr);
--
2.17.1
^ permalink raw reply related
* [PATCH 0/6 bpf-next] xsk: reuseq cleanup
From: Jonathan Lemon @ 2019-06-27 22:08 UTC (permalink / raw)
To: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer; +Cc: kernel-team
Clean up and normalize usage of the recycle queue in order to
support upcoming TX from RX queue functionality.
Jonathan Lemon (6):
Have xsk_umem_peek_addr_rq() return chunk-aligned handles.
Clean up xsk reuseq API
Always check the recycle stack when using the umem fq.
Simplify AF_XDP umem allocation path for Intel drivers.
Remove use of umem _rq variants from Mellanox driver.
Remove the umem _rq variants now that the last consumer is gone.
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 86 +++----------------
.../ethernet/intel/ixgbe/ixgbe_txrx_common.h | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 59 ++-----------
.../ethernet/mellanox/mlx5/core/en/xsk/rx.c | 8 +-
.../ethernet/mellanox/mlx5/core/en/xsk/umem.c | 7 +-
include/net/xdp_sock.h | 69 ++-------------
net/xdp/xdp_umem.c | 2 +-
net/xdp/xsk.c | 22 ++++-
net/xdp/xsk_queue.c | 56 +++++-------
net/xdp/xsk_queue.h | 2 +-
10 files changed, 68 insertions(+), 245 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH 4/6 bfp-next] Simplify AF_XDP umem allocation path for Intel drivers.
From: Jonathan Lemon @ 2019-06-27 22:08 UTC (permalink / raw)
To: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer; +Cc: kernel-team
In-Reply-To: <20190627220836.2572684-1-jonathan.lemon@gmail.com>
Now that the recycle stack is always used for the driver umem path, the
driver code path can be simplified.
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 76 ++-----------------
.../ethernet/intel/ixgbe/ixgbe_txrx_common.h | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 49 +-----------
3 files changed, 13 insertions(+), 114 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 7efe5905b0af..ce8650d06962 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -265,44 +265,16 @@ static bool i40e_alloc_buffer_zc(struct i40e_ring *rx_ring,
}
/**
- * i40e_alloc_buffer_slow_zc - Allocates an i40e_rx_buffer
+ * i40e_alloc_rx_buffers_zc - Allocates a number of Rx buffers
* @rx_ring: Rx ring
- * @bi: Rx buffer to populate
+ * @count: The number of buffers to allocate
*
- * This function allocates an Rx buffer. The buffer can come from fill
- * queue, or via the reuse queue.
+ * This function allocates a number of Rx buffers from the fill ring
+ * or the internal recycle mechanism and places them on the Rx ring.
*
* Returns true for a successful allocation, false otherwise
**/
-static bool i40e_alloc_buffer_slow_zc(struct i40e_ring *rx_ring,
- struct i40e_rx_buffer *bi)
-{
- struct xdp_umem *umem = rx_ring->xsk_umem;
- u64 handle, hr;
-
- if (!xsk_umem_peek_addr_rq(umem, &handle)) {
- rx_ring->rx_stats.alloc_page_failed++;
- return false;
- }
-
- hr = umem->headroom + XDP_PACKET_HEADROOM;
-
- bi->dma = xdp_umem_get_dma(umem, handle);
- bi->dma += hr;
-
- bi->addr = xdp_umem_get_data(umem, handle);
- bi->addr += hr;
-
- bi->handle = handle + umem->headroom;
-
- xsk_umem_discard_addr_rq(umem);
- return true;
-}
-
-static __always_inline bool
-__i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count,
- bool alloc(struct i40e_ring *rx_ring,
- struct i40e_rx_buffer *bi))
+bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count)
{
u16 ntu = rx_ring->next_to_use;
union i40e_rx_desc *rx_desc;
@@ -312,7 +284,7 @@ __i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count,
rx_desc = I40E_RX_DESC(rx_ring, ntu);
bi = &rx_ring->rx_bi[ntu];
do {
- if (!alloc(rx_ring, bi)) {
+ if (!i40e_alloc_buffer_zc(rx_ring, bi)) {
ok = false;
goto no_buffers;
}
@@ -344,38 +316,6 @@ __i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count,
return ok;
}
-/**
- * i40e_alloc_rx_buffers_zc - Allocates a number of Rx buffers
- * @rx_ring: Rx ring
- * @count: The number of buffers to allocate
- *
- * This function allocates a number of Rx buffers from the reuse queue
- * or fill ring and places them on the Rx ring.
- *
- * Returns true for a successful allocation, false otherwise
- **/
-bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count)
-{
- return __i40e_alloc_rx_buffers_zc(rx_ring, count,
- i40e_alloc_buffer_slow_zc);
-}
-
-/**
- * i40e_alloc_rx_buffers_fast_zc - Allocates a number of Rx buffers
- * @rx_ring: Rx ring
- * @count: The number of buffers to allocate
- *
- * This function allocates a number of Rx buffers from the fill ring
- * or the internal recycle mechanism and places them on the Rx ring.
- *
- * Returns true for a successful allocation, false otherwise
- **/
-static bool i40e_alloc_rx_buffers_fast_zc(struct i40e_ring *rx_ring, u16 count)
-{
- return __i40e_alloc_rx_buffers_zc(rx_ring, count,
- i40e_alloc_buffer_zc);
-}
-
/**
* i40e_get_rx_buffer_zc - Return the current Rx buffer
* @rx_ring: Rx ring
@@ -541,8 +481,8 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
if (cleaned_count >= I40E_RX_BUFFER_WRITE) {
failure = failure ||
- !i40e_alloc_rx_buffers_fast_zc(rx_ring,
- cleaned_count);
+ !i40e_alloc_rx_buffers_zc(rx_ring,
+ cleaned_count);
cleaned_count = 0;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
index d93a690aff74..7408fbc2e1e1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
@@ -35,7 +35,7 @@ int ixgbe_xsk_umem_setup(struct ixgbe_adapter *adapter, struct xdp_umem *umem,
void ixgbe_zca_free(struct zero_copy_allocator *alloc, unsigned long handle);
-void ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 cleaned_count);
+bool ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 cleaned_count);
int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
struct ixgbe_ring *rx_ring,
const int budget);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 1e09fa7ffb90..65feb16200ea 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -278,35 +278,7 @@ static bool ixgbe_alloc_buffer_zc(struct ixgbe_ring *rx_ring,
return true;
}
-static bool ixgbe_alloc_buffer_slow_zc(struct ixgbe_ring *rx_ring,
- struct ixgbe_rx_buffer *bi)
-{
- struct xdp_umem *umem = rx_ring->xsk_umem;
- u64 handle, hr;
-
- if (!xsk_umem_peek_addr_rq(umem, &handle)) {
- rx_ring->rx_stats.alloc_rx_page_failed++;
- return false;
- }
-
- hr = umem->headroom + XDP_PACKET_HEADROOM;
-
- bi->dma = xdp_umem_get_dma(umem, handle);
- bi->dma += hr;
-
- bi->addr = xdp_umem_get_data(umem, handle);
- bi->addr += hr;
-
- bi->handle = handle + umem->headroom;
-
- xsk_umem_discard_addr_rq(umem);
- return true;
-}
-
-static __always_inline bool
-__ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 cleaned_count,
- bool alloc(struct ixgbe_ring *rx_ring,
- struct ixgbe_rx_buffer *bi))
+bool ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 cleaned_count)
{
union ixgbe_adv_rx_desc *rx_desc;
struct ixgbe_rx_buffer *bi;
@@ -322,7 +294,7 @@ __ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 cleaned_count,
i -= rx_ring->count;
do {
- if (!alloc(rx_ring, bi)) {
+ if (!ixgbe_alloc_buffer_zc(rx_ring, bi)) {
ok = false;
break;
}
@@ -373,19 +345,6 @@ __ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 cleaned_count,
return ok;
}
-void ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 count)
-{
- __ixgbe_alloc_rx_buffers_zc(rx_ring, count,
- ixgbe_alloc_buffer_slow_zc);
-}
-
-static bool ixgbe_alloc_rx_buffers_fast_zc(struct ixgbe_ring *rx_ring,
- u16 count)
-{
- return __ixgbe_alloc_rx_buffers_zc(rx_ring, count,
- ixgbe_alloc_buffer_zc);
-}
-
static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
struct ixgbe_rx_buffer *bi,
struct xdp_buff *xdp)
@@ -441,8 +400,8 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
/* return some buffers to hardware, one at a time is too slow */
if (cleaned_count >= IXGBE_RX_BUFFER_WRITE) {
failure = failure ||
- !ixgbe_alloc_rx_buffers_fast_zc(rx_ring,
- cleaned_count);
+ !ixgbe_alloc_rx_buffers_zc(rx_ring,
+ cleaned_count);
cleaned_count = 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 1/6 bpf-next] Have xsk_umem_peek_addr_rq() return chunk-aligned handles.
From: Jonathan Lemon @ 2019-06-27 22:08 UTC (permalink / raw)
To: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer; +Cc: kernel-team
In-Reply-To: <20190627220836.2572684-1-jonathan.lemon@gmail.com>
xkq_peek_addr() returns chunk-aligned handles, so have the rq behave
the same way. Clean up callsites.
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 --
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 2 --
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c | 2 +-
include/net/xdp_sock.h | 2 +-
4 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 32bad014d76c..3b84fca1c11d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -289,8 +289,6 @@ static bool i40e_alloc_buffer_slow_zc(struct i40e_ring *rx_ring,
return false;
}
- handle &= rx_ring->xsk_umem->chunk_mask;
-
hr = umem->headroom + XDP_PACKET_HEADROOM;
bi->dma = xdp_umem_get_dma(umem, handle);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 6b609553329f..a9cab3271ac9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -293,8 +293,6 @@ static bool ixgbe_alloc_buffer_slow_zc(struct ixgbe_ring *rx_ring,
return false;
}
- handle &= rx_ring->xsk_umem->chunk_mask;
-
hr = umem->headroom + XDP_PACKET_HEADROOM;
bi->dma = xdp_umem_get_dma(umem, handle);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
index 6a55573ec8f2..85440b1c1c3f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
@@ -44,7 +44,7 @@ int mlx5e_xsk_page_alloc_umem(struct mlx5e_rq *rq,
static inline void mlx5e_xsk_recycle_frame(struct mlx5e_rq *rq, u64 handle)
{
- xsk_umem_fq_reuse(rq->umem, handle & rq->umem->chunk_mask);
+ xsk_umem_fq_reuse(rq->umem, handle);
}
/* XSKRQ uses pages from UMEM, they must not be released. They are returned to
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 057b159ff8b9..39516d960636 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -117,7 +117,7 @@ static inline u64 *xsk_umem_peek_addr_rq(struct xdp_umem *umem, u64 *addr)
if (!rq->length)
return xsk_umem_peek_addr(umem, addr);
- *addr = rq->handles[rq->length - 1];
+ *addr = rq->handles[rq->length - 1] & umem->chunk_mask;
return addr;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 2/6 bpf-next] Clean up xsk reuseq API
From: Jonathan Lemon @ 2019-06-27 22:08 UTC (permalink / raw)
To: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer; +Cc: kernel-team
In-Reply-To: <20190627220836.2572684-1-jonathan.lemon@gmail.com>
The reuseq is actually a recycle stack, only accessed from the kernel side.
Also, the implementation details of the stack should belong to the umem
object, and not exposed to the caller.
Clean up and rename for consistency in preparation for the next patch.
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 8 +--
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 8 +--
.../ethernet/mellanox/mlx5/core/en/xsk/rx.c | 2 +-
.../ethernet/mellanox/mlx5/core/en/xsk/umem.c | 7 +--
include/net/xdp_sock.h | 23 ++------
net/xdp/xdp_umem.c | 2 +-
net/xdp/xsk_queue.c | 56 +++++++------------
net/xdp/xsk_queue.h | 2 +-
8 files changed, 33 insertions(+), 75 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 3b84fca1c11d..7efe5905b0af 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -78,7 +78,6 @@ static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
u16 qid)
{
struct net_device *netdev = vsi->netdev;
- struct xdp_umem_fq_reuse *reuseq;
bool if_running;
int err;
@@ -92,12 +91,9 @@ static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
qid >= netdev->real_num_tx_queues)
return -EINVAL;
- reuseq = xsk_reuseq_prepare(vsi->rx_rings[0]->count);
- if (!reuseq)
+ if (!xsk_umem_recycle_alloc(umem, vsi->rx_rings[0]->count))
return -ENOMEM;
- xsk_reuseq_free(xsk_reuseq_swap(umem, reuseq));
-
err = i40e_xsk_umem_dma_map(vsi, umem);
if (err)
return err;
@@ -811,7 +807,7 @@ void i40e_xsk_clean_rx_ring(struct i40e_ring *rx_ring)
if (!rx_bi->addr)
continue;
- xsk_umem_fq_reuse(rx_ring->xsk_umem, rx_bi->handle);
+ xsk_umem_recycle_addr(rx_ring->xsk_umem, rx_bi->handle);
rx_bi->addr = NULL;
}
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index a9cab3271ac9..1e09fa7ffb90 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -67,7 +67,6 @@ static int ixgbe_xsk_umem_enable(struct ixgbe_adapter *adapter,
u16 qid)
{
struct net_device *netdev = adapter->netdev;
- struct xdp_umem_fq_reuse *reuseq;
bool if_running;
int err;
@@ -78,12 +77,9 @@ static int ixgbe_xsk_umem_enable(struct ixgbe_adapter *adapter,
qid >= netdev->real_num_tx_queues)
return -EINVAL;
- reuseq = xsk_reuseq_prepare(adapter->rx_ring[0]->count);
- if (!reuseq)
+ if (!xsk_umem_recycle_alloc(umem, adapter->rx_ring[0]->count))
return -ENOMEM;
- xsk_reuseq_free(xsk_reuseq_swap(umem, reuseq));
-
err = ixgbe_xsk_umem_dma_map(adapter, umem);
if (err)
return err;
@@ -554,7 +550,7 @@ void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring)
struct ixgbe_rx_buffer *bi = &rx_ring->rx_buffer_info[i];
while (i != rx_ring->next_to_alloc) {
- xsk_umem_fq_reuse(rx_ring->xsk_umem, bi->handle);
+ xsk_umem_recycle_addr(rx_ring->xsk_umem, bi->handle);
i++;
bi++;
if (i == rx_ring->count) {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
index 85440b1c1c3f..8d24eaa660a8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
@@ -44,7 +44,7 @@ int mlx5e_xsk_page_alloc_umem(struct mlx5e_rq *rq,
static inline void mlx5e_xsk_recycle_frame(struct mlx5e_rq *rq, u64 handle)
{
- xsk_umem_fq_reuse(rq->umem, handle);
+ xsk_umem_recycle_addr(rq->umem, handle);
}
/* XSKRQ uses pages from UMEM, they must not be released. They are returned to
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/umem.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/umem.c
index 4baaa5788320..6f75ac653697 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/umem.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/umem.c
@@ -243,13 +243,8 @@ int mlx5e_xsk_setup_umem(struct net_device *dev, struct xdp_umem *umem, u16 qid)
int mlx5e_xsk_resize_reuseq(struct xdp_umem *umem, u32 nentries)
{
- struct xdp_umem_fq_reuse *reuseq;
-
- reuseq = xsk_reuseq_prepare(nentries);
- if (unlikely(!reuseq))
+ if (!xsk_umem_recycle_alloc(umem, nentries))
return -ENOMEM;
- xsk_reuseq_free(xsk_reuseq_swap(umem, reuseq));
-
return 0;
}
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 39516d960636..7df7b417ac53 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -83,10 +83,7 @@ void xsk_umem_discard_addr(struct xdp_umem *umem);
void xsk_umem_complete_tx(struct xdp_umem *umem, u32 nb_entries);
bool xsk_umem_consume_tx(struct xdp_umem *umem, struct xdp_desc *desc);
void xsk_umem_consume_tx_done(struct xdp_umem *umem);
-struct xdp_umem_fq_reuse *xsk_reuseq_prepare(u32 nentries);
-struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
- struct xdp_umem_fq_reuse *newq);
-void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq);
+bool xsk_umem_recycle_alloc(struct xdp_umem *umem, u32 nentries);
struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev, u16 queue_id);
static inline char *xdp_umem_get_data(struct xdp_umem *umem, u64 addr)
@@ -131,7 +128,7 @@ static inline void xsk_umem_discard_addr_rq(struct xdp_umem *umem)
rq->length--;
}
-static inline void xsk_umem_fq_reuse(struct xdp_umem *umem, u64 addr)
+static inline void xsk_umem_recycle_addr(struct xdp_umem *umem, u64 addr)
{
struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
@@ -185,19 +182,9 @@ static inline void xsk_umem_consume_tx_done(struct xdp_umem *umem)
{
}
-static inline struct xdp_umem_fq_reuse *xsk_reuseq_prepare(u32 nentries)
-{
- return NULL;
-}
-
-static inline struct xdp_umem_fq_reuse *xsk_reuseq_swap(
- struct xdp_umem *umem,
- struct xdp_umem_fq_reuse *newq)
-{
- return NULL;
-}
-static inline void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq)
+static inline bool xsk_umem_recycle_alloc(struct xdp_umem *umem, u32 nentries)
{
+ return false;
}
static inline struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev,
@@ -230,7 +217,7 @@ static inline void xsk_umem_discard_addr_rq(struct xdp_umem *umem)
{
}
-static inline void xsk_umem_fq_reuse(struct xdp_umem *umem, u64 addr)
+static inline void xsk_umem_recycle_addr(struct xdp_umem *umem, u64 addr)
{
}
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 9c6de4f114f8..2150ba3fc744 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -208,7 +208,7 @@ static void xdp_umem_release(struct xdp_umem *umem)
umem->cq = NULL;
}
- xsk_reuseq_destroy(umem);
+ xsk_umem_recycle_destroy(umem);
xdp_umem_unpin_pages(umem);
diff --git a/net/xdp/xsk_queue.c b/net/xdp/xsk_queue.c
index b66504592d9b..116b28622297 100644
--- a/net/xdp/xsk_queue.c
+++ b/net/xdp/xsk_queue.c
@@ -65,55 +65,39 @@ void xskq_destroy(struct xsk_queue *q)
kfree(q);
}
-struct xdp_umem_fq_reuse *xsk_reuseq_prepare(u32 nentries)
+bool xsk_umem_recycle_alloc(struct xdp_umem *umem, u32 nentries)
{
- struct xdp_umem_fq_reuse *newq;
+ struct xdp_umem_fq_reuse *newq, *oldq;
/* Check for overflow */
if (nentries > (u32)roundup_pow_of_two(nentries))
- return NULL;
+ return false;
nentries = roundup_pow_of_two(nentries);
+ oldq = umem->fq_reuse;
+ if (oldq && (oldq->length >= nentries ||
+ oldq->nentries == nentries))
+ return true;
newq = kvmalloc(struct_size(newq, handles, nentries), GFP_KERNEL);
if (!newq)
- return NULL;
- memset(newq, 0, offsetof(typeof(*newq), handles));
-
- newq->nentries = nentries;
- return newq;
-}
-EXPORT_SYMBOL_GPL(xsk_reuseq_prepare);
-
-struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
- struct xdp_umem_fq_reuse *newq)
-{
- struct xdp_umem_fq_reuse *oldq = umem->fq_reuse;
-
- if (!oldq) {
- umem->fq_reuse = newq;
- return NULL;
+ return false;
+ if (oldq) {
+ memcpy(newq->handles, oldq->handles,
+ array_size(oldq->length, sizeof(u64)));
+ newq->length = oldq->length;
+ kvfree(oldq);
+ } else {
+ newq->length = 0;
}
-
- if (newq->nentries < oldq->length)
- return newq;
-
- memcpy(newq->handles, oldq->handles,
- array_size(oldq->length, sizeof(u64)));
- newq->length = oldq->length;
-
+ newq->nentries = nentries;
umem->fq_reuse = newq;
- return oldq;
-}
-EXPORT_SYMBOL_GPL(xsk_reuseq_swap);
-void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq)
-{
- kvfree(rq);
+ return true;
}
-EXPORT_SYMBOL_GPL(xsk_reuseq_free);
+EXPORT_SYMBOL_GPL(xsk_umem_recycle_alloc);
-void xsk_reuseq_destroy(struct xdp_umem *umem)
+void xsk_umem_recycle_destroy(struct xdp_umem *umem)
{
- xsk_reuseq_free(umem->fq_reuse);
+ kvfree(umem->fq_reuse);
umem->fq_reuse = NULL;
}
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 12b49784a6d5..c9db4e8ae80e 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -321,6 +321,6 @@ struct xsk_queue *xskq_create(u32 nentries, bool umem_queue);
void xskq_destroy(struct xsk_queue *q_ops);
/* Executed by the core when the entire UMEM gets freed */
-void xsk_reuseq_destroy(struct xdp_umem *umem);
+void xsk_umem_recycle_destroy(struct xdp_umem *umem);
#endif /* _LINUX_XSK_QUEUE_H */
--
2.17.1
^ permalink raw reply related
* [PATCH 5/6 bpf-next] Remove use of umem _rq variants from Mellanox driver.
From: Jonathan Lemon @ 2019-06-27 22:08 UTC (permalink / raw)
To: netdev, bjorn.topel, magnus.karlsson, saeedm, maximmi, brouer; +Cc: kernel-team
In-Reply-To: <20190627220836.2572684-1-jonathan.lemon@gmail.com>
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
index 8d24eaa660a8..e116b1fde797 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/rx.c
@@ -12,7 +12,7 @@ bool mlx5e_xsk_pages_enough_umem(struct mlx5e_rq *rq, int count)
/* Check in advance that we have enough frames, instead of allocating
* one-by-one, failing and moving frames to the Reuse Ring.
*/
- return xsk_umem_has_addrs_rq(rq->umem, count);
+ return xsk_umem_has_addrs(rq->umem, count);
}
int mlx5e_xsk_page_alloc_umem(struct mlx5e_rq *rq,
@@ -21,7 +21,7 @@ int mlx5e_xsk_page_alloc_umem(struct mlx5e_rq *rq,
struct xdp_umem *umem = rq->umem;
u64 handle;
- if (!xsk_umem_peek_addr_rq(umem, &handle))
+ if (!xsk_umem_peek_addr(umem, &handle))
return -ENOMEM;
dma_info->xsk.handle = handle + rq->buff.umem_headroom;
@@ -34,7 +34,7 @@ int mlx5e_xsk_page_alloc_umem(struct mlx5e_rq *rq,
*/
dma_info->addr = xdp_umem_get_dma(umem, handle);
- xsk_umem_discard_addr_rq(umem);
+ xsk_umem_discard_addr(umem);
dma_sync_single_for_device(rq->pdev, dma_info->addr, PAGE_SIZE,
DMA_BIDIRECTIONAL);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Daniel Borkmann @ 2019-06-27 22:08 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <156125626136.5209.14349225282974871197.stgit@alrua-x1>
On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> The bpf_redirect_map() helper used by XDP programs doesn't return any
> indication of whether it can successfully redirect to the map index it was
> given. Instead, BPF programs have to track this themselves, leading to
> programs using duplicate maps to track which entries are populated in the
> devmap.
>
> This patch fixes this by moving the map lookup into the bpf_redirect_map()
> helper, which makes it possible to return failure to the eBPF program. The
> lower bits of the flags argument is used as the return code, which means
> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>
> With this, a BPF program can check the return code from the helper call and
> react by, for instance, substituting a different redirect. This works for
> any type of map used for redirect.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
[...]
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 183bf4d8e301..a6779e1cc1b8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3605,17 +3605,13 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
> struct bpf_redirect_info *ri)
> {
> u32 index = ri->ifindex;
> - void *fwd = NULL;
> + void *fwd = ri->item;
> int err;
>
> ri->ifindex = 0;
> + ri->item = NULL;
> WRITE_ONCE(ri->map, NULL);
>
> - fwd = __xdp_map_lookup_elem(map, index);
> - if (unlikely(!fwd)) {
> - err = -EINVAL;
> - goto err;
> - }
If you look at the _trace_xdp_redirect{,_err}(), we should also get rid of the
extra NULL test in devmap_ifindex() which is not under tracepoint static key.
^ permalink raw reply
* Re: [PATCH net] net/sched: flower: fix infinite loop in fl_walk()
From: Davide Caratti @ 2019-06-27 22:10 UTC (permalink / raw)
To: Cong Wang
Cc: Vlad Buslov, David S. Miller, Linux Kernel Network Developers,
Lucas Bates
In-Reply-To: <CAM_iQpV8Euk=NT4M7R5mAoS6_zU7aWBLRtkKEMatCxLAyaxSjQ@mail.gmail.com>
On Wed, 2019-06-26 at 14:15 -0700, Cong Wang wrote:
> Hi, Davide
>
> On Tue, Jun 25, 2019 at 12:29 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > It should handle this overflow case more gracefully, I hope.
> >
>
> Please try this attached one and let me know if it works.
> Hope I get it right this time.
>
> Thanks!
hello Cong, and thanks a lot for the patch!
I see it uses
(tmp <= id)
as the condition to detect the overflow, and at each iteration it does
tmp = id, ++id
so that 'tmp' contains the last IDR found in the tree and 'id' is the next
tentative value to be searched for. When 'id' overflows, (tmp <= id)
becomes false, and the 'for' loop exits.
I tested it successfully with TC actions having the highest possible
index: 'tc actions show' doesn't loop anymore. But with cls_flower (that
uses idr_for_each_entry_continue_ul() ) I still see the infinite loop:
even when idr_for_each_entry_continue_ul() is used, fl_get_next_filter()
never returns NULL, because
(tmp <= id) && (((entry) = idr_get_next_ul(idr, &(id))) != NULL)
calls idr_get_next_ul(idr, &(id)) at least once. So, even if
idr_for_each_entry_continue_ul() detected the overflow of 'id' after the
first iteration, and bailouts the for loop, fl_get_next_filter()
repeatedly returns a pointer to the idr slot with index equal to
0xffffffff. Because of that, the while() loop in fl_walk() keeps dumping
the same rule.
In my original patch I found easier to check for the overflow of
arg->cookie in fl_walk(), before the self-increment, so I was sure that
arg->fn(tp, f, arg)
was already called once when 'f' was the slot having the highest possible
IDR. Now, I didn't check it, but I guess
refcount_inc_not_zero(&f->refcnt))
in fl_get_next_filter() is always true during my test, so the inner
while() loop is not endless, even when the idr has a slot with id equal to
ULONG_MAX. Probably, to stay on the safe side, cls_flower needs both tests
to be in place, what do you think?
--
davide
^ permalink raw reply
* Re: [RFC PATCH bpf-next v2 2/6] bpf: add BPF_MAP_DUMP command to access more than one entry per call
From: Alexei Starovoitov @ 2019-06-27 22:12 UTC (permalink / raw)
To: Brian Vazquez
Cc: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller, Stanislav Fomichev, Willem de Bruijn,
Petar Penkov, linux-kernel, netdev, bpf
In-Reply-To: <20190627202417.33370-3-brianvv@google.com>
On Thu, Jun 27, 2019 at 01:24:13PM -0700, Brian Vazquez wrote:
> This introduces a new command to retrieve a variable number of entries
> from a bpf map wrapping the existing bpf methods:
> map_get_next_key and map_lookup_elem
>
> Note that map_dump doesn't guarantee that reading the entire table is
> consistent since this function is always racing with kernel and user code
> but the same behaviour is found when the entire table is walked using
> the current interfaces: map_get_next_key + map_lookup_elem.
> It is also important to note that when a locked map is provided it is
> consistent only for 1 entry at the time, meaning that the buf returned
> might or might not be consistent.
Please explain the api behavior and corner cases in the commit log
or in code comments.
Would it make sense to return last key back into prev_key,
so that next map_dump command doesn't need to copy it from the
buffer?
> Suggested-by: Stanislav Fomichev <sdf@google.com>
> Signed-off-by: Brian Vazquez <brianvv@google.com>
> ---
> include/uapi/linux/bpf.h | 9 ++++
> kernel/bpf/syscall.c | 108 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 117 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index b077507efa3f3..1d753958874df 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -106,6 +106,7 @@ enum bpf_cmd {
> BPF_TASK_FD_QUERY,
> BPF_MAP_LOOKUP_AND_DELETE_ELEM,
> BPF_MAP_FREEZE,
> + BPF_MAP_DUMP,
> };
>
> enum bpf_map_type {
> @@ -385,6 +386,14 @@ union bpf_attr {
> __u64 flags;
> };
>
> + struct { /* struct used by BPF_MAP_DUMP command */
> + __u32 map_fd;
> + __aligned_u64 prev_key;
> + __aligned_u64 buf;
> + __aligned_u64 buf_len; /* input/output: len of buf */
> + __u64 flags;
> + } dump;
> +
> struct { /* anonymous struct used by BPF_PROG_LOAD command */
> __u32 prog_type; /* one of enum bpf_prog_type */
> __u32 insn_cnt;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index a1823a50f9be0..7653346b5cfd1 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1097,6 +1097,111 @@ static int map_get_next_key(union bpf_attr *attr)
> return err;
> }
>
> +/* last field in 'union bpf_attr' used by this command */
> +#define BPF_MAP_DUMP_LAST_FIELD dump.buf_len
> +
> +static int map_dump(union bpf_attr *attr)
> +{
> + void __user *ukey = u64_to_user_ptr(attr->dump.prev_key);
> + void __user *ubuf = u64_to_user_ptr(attr->dump.buf);
> + u32 __user *ubuf_len = u64_to_user_ptr(attr->dump.buf_len);
> + int ufd = attr->dump.map_fd;
> + struct bpf_map *map;
> + void *buf, *prev_key, *key, *value;
> + u32 value_size, elem_size, buf_len, cp_len;
> + struct fd f;
> + int err;
> +
> + if (CHECK_ATTR(BPF_MAP_DUMP))
> + return -EINVAL;
> +
> + attr->flags = 0;
> + if (attr->dump.flags & ~BPF_F_LOCK)
> + return -EINVAL;
> +
> + f = fdget(ufd);
> + map = __bpf_map_get(f);
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> + if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ)) {
> + err = -EPERM;
> + goto err_put;
> + }
> +
> + if ((attr->dump.flags & BPF_F_LOCK) &&
> + !map_value_has_spin_lock(map)) {
> + err = -EINVAL;
> + goto err_put;
> + }
> +
> + if (map->map_type == BPF_MAP_TYPE_QUEUE ||
> + map->map_type == BPF_MAP_TYPE_STACK) {
> + err = -ENOTSUPP;
> + goto err_put;
> + }
> +
> + value_size = bpf_map_value_size(map);
> +
> + err = get_user(buf_len, ubuf_len);
> + if (err)
> + goto err_put;
> +
> + elem_size = map->key_size + value_size;
> + if (buf_len < elem_size) {
> + err = -EINVAL;
> + goto err_put;
> + }
> +
> + if (ukey) {
> + prev_key = __bpf_copy_key(ukey, map->key_size);
> + if (IS_ERR(prev_key)) {
> + err = PTR_ERR(prev_key);
> + goto err_put;
> + }
> + } else {
> + prev_key = NULL;
> + }
> +
> + err = -ENOMEM;
> + buf = kmalloc(elem_size, GFP_USER | __GFP_NOWARN);
> + if (!buf)
> + goto err_put;
> +
> + key = buf;
> + value = key + map->key_size;
> + for (cp_len = 0; cp_len + elem_size <= buf_len ; cp_len += elem_size) {
checkpatch.pl please.
> +next:
> + if (signal_pending(current)) {
> + err = -EINTR;
> + break;
> + }
> +
> + rcu_read_lock();
> + err = map->ops->map_get_next_key(map, prev_key, key);
> + rcu_read_unlock();
> +
> + if (err)
> + break;
should probably be only for ENOENT case?
and other errors should be returned to user ?
> +
> + if (bpf_map_copy_value(map, key, value, attr->dump.flags))
> + goto next;
only for ENOENT as well?
and instead of goto use continue and move cp_len+= to the end after prev_key=key?
> +
> + if (copy_to_user(ubuf + cp_len, buf, elem_size))
> + break;
return error to user?
> +
> + prev_key = key;
> + }
> +
> + if (cp_len)
> + err = 0;
this will mask any above errors if there was at least one element copied.
> + if (copy_to_user(ubuf_len, &cp_len, sizeof(cp_len)))
> + err = -EFAULT;
> + kfree(buf);
> +err_put:
> + fdput(f);
> + return err;
> +}
> +
> #define BPF_MAP_LOOKUP_AND_DELETE_ELEM_LAST_FIELD value
>
> static int map_lookup_and_delete_elem(union bpf_attr *attr)
> @@ -2891,6 +2996,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
> case BPF_MAP_LOOKUP_AND_DELETE_ELEM:
> err = map_lookup_and_delete_elem(&attr);
> break;
> + case BPF_MAP_DUMP:
> + err = map_dump(&attr);
> + break;
> default:
> err = -EINVAL;
> break;
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ 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