* [PATCH net-next] r8169: remove not needed call to dma_sync_single_for_device
From: Heiner Kallweit @ 2019-06-27 21:19 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org
DMA_API_HOWTO.txt includes an example explaining when
dma_sync_single_for_device() is not needed, and that example matches
our use case. The buffer isn't changed by the CPU and direction is
DMA_FROM_DEVICE, so we can remove the call to
dma_sync_single_for_device().
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169_main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 1b5f125e1..a73f25321 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5785,7 +5785,6 @@ static struct sk_buff *rtl8169_try_rx_copy(void *data,
skb = napi_alloc_skb(&tp->napi, pkt_size);
if (skb)
skb_copy_to_linear_data(skb, data, pkt_size);
- dma_sync_single_for_device(d, addr, pkt_size, DMA_FROM_DEVICE);
return skb;
}
--
2.22.0
^ permalink raw reply related
* Re: [PATCH 00/11] XDP unaligned chunk placement support
From: Jakub Kicinski @ 2019-06-27 21:25 UTC (permalink / raw)
To: Laatz, Kevin
Cc: Jonathan Lemon, netdev, ast, daniel, bjorn.topel, magnus.karlsson,
bpf, intel-wired-lan, bruce.richardson, ciara.loftus
In-Reply-To: <ef7e9469-e7be-647b-8bb1-da29bc01fa2e@intel.com>
On Thu, 27 Jun 2019 12:14:50 +0100, Laatz, Kevin wrote:
> On the application side (xdpsock), we don't have to worry about the user
> defined headroom, since it is 0, so we only need to account for the
> XDP_PACKET_HEADROOM when computing the original address (in the default
> scenario).
That assumes specific layout for the data inside the buffer. Some NICs
will prepend information like timestamp to the packet, meaning the
packet would start at offset XDP_PACKET_HEADROOM + metadata len..
I think that's very limiting. What is the challenge in providing
aligned addresses, exactly?
^ permalink raw reply
* [Patch net] netrom: fix a memory leak in nr_rx_frame()
From: Cong Wang @ 2019-06-27 21:30 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, syzbot+d6636a36d3c34bd88938
When the skb is associated with a new sock, just assigning
it to skb->sk is not sufficient, we have to set its destructor
to free the sock properly too.
Reported-by: syzbot+d6636a36d3c34bd88938@syzkaller.appspotmail.com
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/netrom/af_netrom.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 86b87925ef34..96740d389377 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -869,7 +869,7 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
unsigned short frametype, flags, window, timeout;
int ret;
- skb->sk = NULL; /* Initially we don't know who it's for */
+ skb_orphan(skb);
/*
* skb->data points to the netrom frame start
@@ -968,6 +968,7 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
window = skb->data[20];
skb->sk = make;
+ skb->destructor = sock_efree;
make->sk_state = TCP_ESTABLISHED;
/* Fill in his circuit details */
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 3/7] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-06-27 21:42 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Andrii Nakryiko, Alexei Starovoitov, Stanislav Fomichev, bpf,
Networking, Kernel Team
In-Reply-To: <44d3b02d-b0fb-b0cb-a0d3-e7dd4bde0b92@iogearbox.net>
On Thu, Jun 27, 2019 at 2:16 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 06/27/2019 12:15 AM, Andrii Nakryiko wrote:
> > On Wed, Jun 26, 2019 at 7:25 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> [...]
> >> What this boils down to is that this should get a proper abstraction, e.g. as
> >> in struct libbpf_event which holds the event object. There should be helper
> >> functions like libbpf_event_create_{kprobe,uprobe,tracepoint,raw_tracepoint} returning
> >> such an struct libbpf_event object on success, and a single libbpf_event_destroy()
> >> that does the event specific teardown. bpf_program__attach_event() can then take
> >> care of only attaching the program to it. Having an object for this is also more
> >> extensible than just a fd number. Nice thing is that this can also be completely
> >> internal to libbpf.c as with struct bpf_program and other abstractions where we
> >> don't expose the internals in the public header.
> >
> > Yeah, I totally agree, I think this is a great idea! I don't
> > particularly like "event" name, that seems very overloaded term. Do
> > you mind if I call this "bpf_hook" instead of "libbpf_event"? I've
> > always thought about these different points in the system to which one
> > can attach BPF program as hooks exposed from kernel :)
> >
> > Would it also make sense to do attaching to non-tracing hooks using
> > the same mechanism (e.g., all the per-cgroup stuff, sysctl, etc)? Not
> > sure how people do that today, will check to see how it's done, but I
> > think nothing should conceptually prevent doing that using the same
> > abstract bpf_hook way, right?
>
> I think if we abstract it this way, then absolutely. If I grok the naming conventions
> from the README right, then this would be under 'bpf_hook__' prefix. :)
Yeah, so this is what I had, API-wise:
struct bpf_hook;
LIBBPF_API struct bpf_hook *bpf_hook__new_perf_event(int pfd);
LIBBPF_API struct bpf_hook *bpf_hook__new_kprobe(bool retprobe,
const char *func_name);
LIBBPF_API struct bpf_hook *bpf_hook__new_uprobe(bool retprobe, pid_t pid,
const char *binary_path,
size_t func_offset);
LIBBPF_API struct bpf_hook *bpf_hook__new_tracepoint(const char *tp_category,
const char *tp_name);
LIBBPF_API struct bpf_hook *bpf_hook__new_raw_tracepoint(const char *tp_name);
LIBBPF_API int bpf_hook__attach_program(struct bpf_hook *hook,
struct bpf_program *prog);
LIBBPF_API int bpf_hook__free(struct bpf_hook *hook);
You'd use bpf_hook_new_xxx to create struct bpf_hook, which then get's
attached using generic bpf_hook__attach_program and detached/freed
with generic bpf_hook__free.
But once I converted selftests, I realized that this generic
bpf_hook__attach_program is kind of unnecessary and is just a
boiler-plate extra function that everyone has to call. So now I'm
leaning towards a hybrid approach:
- bpf_program__attach_xxx will create some specific struct bpf_hook
*and* attach bpf_program to it;
- bpf_hook__free(struct bpf_hook *) would still be used to detach/free
resources, abstracting away specifics of detaching.
There are few benefits to this, I think:
1. Less error checking and clean up from caller: attach either
succeeds (and you'll have to eventually do bpf_hook__free) or not, and
then nothing needs to be cleaned up. With separate create/attach, if
create succeeds, but attach fails, you'll have to do extra
bpf_hook__free call. This bundling of create/attach does prevent
theoretical use case of having hook creation in one place and then
pass this generically into another place for attachment, but it seems
like a bit far-fetched corner use case, which can be implemented at
application level, if necessary.
2. bpf_program__attach has more context for helpful log messages, if
something goes wrong. E.g.,
bpf_program__attach_tracepoint(tp_category, tp_name), once it created
perf event FD for tracepoint, can discard tp_category and tp_name and
use only FD for attachment. But if attachment fails, we don't really
know which tracepoint failed to attach. To facilitate that, you'd need
to allocate/copy tp_category/tp_name (just in case for logging), which
is PITA. With bundled attach, you can log nice error with context
right there with no overhead.
This still allows to do cgroup/flow/sysctl/etc attachment in similar
uniform way (but that's for another set of patches).
Also, I renamed bpf_hook to bpf_link as it seems to convey connection
between connection point (hook) and bpf_program better. Alternative
might be bpf_assoc, I'll mention that as well in cover letter.
Anyways, I think it's better usability without losing anything
(realistically) in terms of flexibility for users. I'll post v2 later
today.
>
> Thanks,
> Daniel
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/3] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-06-27 21:45 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Networking, Kernel Team
In-Reply-To: <7de14b2b-a663-eed9-8f70-fb6bd5ea84d8@iogearbox.net>
On Thu, Jun 27, 2019 at 2:04 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 06/26/2019 08:12 AM, Andrii Nakryiko wrote:
> > BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
> > to user space for additional processing. libbpf already has very low-level API
> > to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
> > use and requires a lot of code to set everything up. This patch adds
> > perf_buffer abstraction on top of it, abstracting setting up and polling
> > per-CPU logic into simple and convenient API, similar to what BCC provides.
> >
> > perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
> > map entries. It accepts two user-provided callbacks: one for handling raw
> > samples and one for get notifications of lost samples due to buffer overflow.
> >
> > perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
> > utilizing epoll instance.
> >
> > perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
> >
> > All APIs are not thread-safe. User should ensure proper locking/coordination if
> > used in multi-threaded set up.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Aside from current feedback, this series generally looks great! Two small things:
>
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 2382fbda4cbb..10f48103110a 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -170,13 +170,16 @@ LIBBPF_0.0.4 {
> > btf_dump__dump_type;
> > btf_dump__free;
> > btf_dump__new;
> > - btf__parse_elf;
> > bpf_object__load_xattr;
> > bpf_program__attach_kprobe;
> > bpf_program__attach_perf_event;
> > bpf_program__attach_raw_tracepoint;
> > bpf_program__attach_tracepoint;
> > bpf_program__attach_uprobe;
> > + btf__parse_elf;
> > libbpf_num_possible_cpus;
> > libbpf_perf_event_disable_and_close;
> > + perf_buffer__free;
> > + perf_buffer__new;
> > + perf_buffer__poll;
>
> We should prefix with libbpf_* given it's not strictly BPF-only and rather
> helper function.
Well, perf_buffer is an object similar to `struct btf`, `struct
bpf_program`, etc. So it seems appropriate to follow this
"<object>__<method>" convention. Also, `struct libbpf_perf_buffer` and
`libbpf_perf_buffer__new` looks verbose and pretty ugly, IMO.
>
> Also, we should convert bpftool (tools/bpf/bpftool/map_perf_ring.c) to make
> use of these new helpers instead of open-coding there.
Yep, absolutely, will do that as well, thanks for pointing me there!
>
> Thanks,
> Daniel
^ permalink raw reply
* [PATCH v2 net-next 0/3] Better PHYLINK compliance for SJA1105 DSA
From: Vladimir Oltean @ 2019-06-27 21:46 UTC (permalink / raw)
To: linux, f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
After discussing with Russell King, it appears this driver is making a
few confusions and not performing some checks for consistent operation.
Changes in v2:
- Removed redundant print in the phylink_validate callback (in 2/3).
Vladimir Oltean (3):
net: dsa: sja1105: Don't check state->link in phylink_mac_config
net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK
reports
net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK
drivers/net/dsa/sja1105/sja1105_main.c | 56 +++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply
* [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
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