* Re: [PATCH net-next 0/2] mlx4 XDP TX refactor
From: Tariq Toukan @ 2016-10-30 16:21 UTC (permalink / raw)
To: Tariq Toukan, David S. Miller; +Cc: netdev, Eran Ben Elisha
In-Reply-To: <1477579924-32737-1-git-send-email-tariqt@mellanox.com>
Hi Dave,
This series makes Brenden's fix unneeded:
958b3d396d7f ("net/mlx4_en: fixup xdp tx irq to match rx")
The fix got into net, but yet to be in net-next.
Should I wait with this series and send a re-spin, with a revert of the
fix, once it gets into net-next?
Regards,
Tariq
On 27/10/2016 5:52 PM, Tariq Toukan wrote:
> Hi Dave,
>
> This patchset refactors the XDP forwarding case, so that
> its dedicated transmit queues are managed in a complete
> separation from the other regular ones.
>
> Series generated against net-next commit:
> 6edf10173a1f "devlink: Prevent port_type_set() callback when it's not needed"
>
> Thanks,
> Tariq.
>
> Tariq Toukan (2):
> net/mlx4_en: Add TX_XDP for CQ types
> net/mlx4_en: Refactor the XDP forwarding rings scheme
>
> drivers/net/ethernet/mellanox/mlx4/en_cq.c | 18 +-
> drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 76 +++--
> drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +-
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 378 ++++++++++++++----------
> drivers/net/ethernet/mellanox/mlx4/en_port.c | 4 +-
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 8 +-
> drivers/net/ethernet/mellanox/mlx4/en_tx.c | 9 +-
> drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 18 +-
> 8 files changed, 305 insertions(+), 208 deletions(-)
>
^ permalink raw reply
* Re: [PATCH net-next 2/2] net/mlx4_en: Refactor the XDP forwarding rings scheme
From: Tariq Toukan @ 2016-10-30 16:03 UTC (permalink / raw)
To: Alexei Starovoitov, Tariq Toukan
Cc: David S. Miller, netdev, Eran Ben Elisha, Brenden Blanco
In-Reply-To: <20161028010701.GB49550@ast-mbp.thefacebook.com>
On 28/10/2016 4:07 AM, Alexei Starovoitov wrote:
> On Thu, Oct 27, 2016 at 05:52:04PM +0300, Tariq Toukan wrote:
>> Separately manage the two types of TX rings: regular ones, and XDP.
>> Upon an XDP set, do not borrow regular TX rings and convert them
>> into XDP ones, but allocate new ones, unless we hit the max number
>> of rings.
>> Which means that in systems with smaller #cores we will not consume
>> the current TX rings for XDP, while we are still in the num TX limit.
> The commit log is too scarce for details...
> So questions:
> - Did you test with changing the number of channels after xdp prog is loaded?
> That was the recent bug that Brenden fixed.
Bug no longer exists, as the indices of the XDP TX rings now start from
0, each is identical to its respective RX ring.
Brenden's fix didn't get to net-next yet, and it shouldn't once the
series is applied.
I need to take this w Dave.
> - does it still have 256 tx queue limit or xdp tx rings can go over?
It still has the limit of 256 TX queues.
> - Any performance implications ?
I didn't see any performance implications.
Note that the XDP TX rings are no longer shown in ethtool -S.
>
> Brenden, could you please review this patch?
>
Regards,
Tariq
^ permalink raw reply
* Re: [PATCH for-next 00/14][PULL request] Mellanox mlx5 core driver updates 2016-10-25
From: David Miller @ 2016-10-30 16:02 UTC (permalink / raw)
To: saeedm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb
Cc: saeedm-VPRAkNaXOzVWk0Htik3J/w, dledford-H+wXaHxf7aLQT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w, leonro-VPRAkNaXOzVWk0Htik3J/w,
talal-VPRAkNaXOzVWk0Htik3J/w, matanb-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <CALzJLG8cN0VUiTHDdkgibObA970UsAP+E7E=DSgY1RKNefSyzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Saeed Mahameed <saeedm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Date: Sun, 30 Oct 2016 11:59:57 +0200
> On Fri, Oct 28, 2016 at 7:53 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
>>
>> I really disalike pull requests of this form.
>>
>> You add lots of datastructures and helper functions but no actual
>> users of these facilities to the driver.
>>
>> Do this instead:
>>
>> 1) Add TSAR infrastructure
>> 2) Add use of TSAR facilities to the driver
>>
>> That's one pull request.
>>
>> I don't care if this is hard, or if there are entanglements with
>> Infiniband or whatever, you must submit changes in this manner.
>>
>
> It is not hard, it is just not right, we have lots of IB and ETH
> features that we would like to submit in the same kernel cycle,
> with your suggestion I will have to almost submit every feature (core
> infrastructure and netdev/RDMA usage)
> to you and Doug.
Nobody can properly review an API addition without seeing how that
API is _USED_.
This is a simple fundamental fact.
And I'm not pulling in code that can't be reviewed properly.
Also, so many times people have added new junk to drivers and months
later never added the users of that new code and interfaces.
Forcing you to provide the use with the API addition makes sure that
it is absolutely impossible for that to happen.
Whatever issues you think prevent this are your issues, not mine. I
want high quality submissions that can be properly reviewed, and you
have to find a way to satisfy that requirement.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower
From: John Heenan @ 2016-10-30 13:56 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <wrfjpomidp1v.fsf@redhat.com>
Thanks for your reply.
The code was tested on a Cube i9 which has an internal rtl8723bu.
No other devices were tested.
I am happy to accept in an ideal context hard coding macpower is
undesirable, the comment is undesirable and it is wrong to assume the
issue is not unique to the rtl8723bu.
Your reply is idealistic. What can I do now? I should of course have
factored out other untested devices in my patches. The apparent
concern you have with process over outcome is a useful lesson.
We are not in an ideal situation. The comment is of course relevant
and useful to starting a process to fixing a real bug I do not have
sufficient information to refine any further for and others do. In the
circumstances nothing really more can be expected.
My patch cover letter, [PATCH 0/2] provides evidence of a mess with
regard to determining macpower for the rtl8723bu and what is
subsequently required. This is important.
The kernel driver code is very poorly documented and there is not a
single source reference to device documentation. For example macpower
is noting more than a setting that is true or false according to
whether a read of a particular register return 0xef or not. Such value
was never obtained so a full init sequence was never performed.
It would be helpful if you could provide a link to device references.
As it is, how am I supposed to revise the patch without relevant
information?
My patch code works with the Cube i9, as is, despite a lack of
adequate information. Before it did not. That is a powerful statement
Have a nice day.
John Heenan
On 30 October 2016 at 22:00, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> John Heenan <john@zgus.com> writes:
>> Code tests show data returned by rtl8xxxu_read8(priv, REG_CR), used to set
>> macpower, is never 0xea. It is only ever 0x01 (first time after modprobe)
>> using wpa_supplicant and 0x00 thereafter using wpa_supplicant. These results
>> occurs with 'Fix for authentication failure' [PATCH 1/2] in place.
>>
>> Whatever was returned, code tests always showed that at least
>> rtl8xxxu_init_queue_reserved_page(priv);
>> is always required. Not called if macpower set to true.
>>
>> Please see cover letter, [PATCH 0/2], for more information from tests.
>>
>
> Sorry but this patch is neither serious nor acceptable. First of all,
> hardcoding macpower like this right after an if statement is plain
> wrong, second your comments violate all kernel rules.
>
> Second, you argue this was tested using code test - on which device? Did
> you test it on all rtl8xxxu based devices or just rtl8723bu?
>
> NACK
>
> Jes
^ permalink raw reply
* Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep
From: Eric Dumazet @ 2016-10-30 13:20 UTC (permalink / raw)
To: Andrey Konovalov, Peter Zijlstra
Cc: Cong Wang, Gerrit Renker, David S. Miller, dccp, netdev, LKML,
Dmitry Vyukov, Eric Dumazet
In-Reply-To: <CAAeHK+z2mqZCsZpPP_hNY2-bt_-WEU9rFmOd=jcWSBmVPDS_rA@mail.gmail.com>
On Sun, 2016-10-30 at 05:41 +0100, Andrey Konovalov wrote:
> Sorry, the warning is still there.
>
> I'm not sure adding sched_annotate_sleep() does anything, since it's
> defined as (in case CONFIG_DEBUG_ATOMIC_SLEEP is not set):
> # define sched_annotate_sleep() do { } while (0)
Thanks again for testing.
But you do have CONFIG_DEBUG_ATOMIC_SLEEP set, which triggers a check in
__might_sleep() :
WARN_ONCE(current->state != TASK_RUNNING && current->task_state_change,
Relevant commit is 00845eb968ead28007338b2bb852b8beef816583
("sched: don't cause task state changes in nested sleep debugging")
Another relevant commit was 26cabd31259ba43f68026ce3f62b78094124333f
("sched, net: Clean up sk_wait_event() vs. might_sleep()")
Before release_sock() could process the backlog in process context, only
lock_sock() could trigger the issue, so my fix at that time was commit
cb7cf8a33ff73cf638481d1edf883d8968f934f8 ("inet: Clean up
inet_csk_wait_for_connect() vs. might_sleep()")
I guess we need something else now, because the following :
static int dccp_wait_for_ccid(struct sock *sk, unsigned long delay)
{
DEFINE_WAIT(wait);
long remaining;
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
sk->sk_write_pending++;
release_sock(sk);
...
can now process the socket backlog in process context from
release_sock(), so all GFP_KERNEL allocations might barf because of
TASK_INTERRUPTIBLE being used at that point.
sk_wait_event() probably also needs a fix.
Peter, any idea how this can be done ?
Thanks !
^ permalink raw reply
* Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
From: Jakub Sitnicki @ 2016-10-30 13:03 UTC (permalink / raw)
To: Tom Herbert
Cc: Linux Kernel Network Developers, LKML, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy
In-Reply-To: <CALx6S35qmAwFfK1TNAcJJs1jK7JgM8U4MCh3wC8hkuf2qae+sw@mail.gmail.com>
On Fri, Oct 28, 2016 at 02:25 PM GMT, Tom Herbert wrote:
> On Fri, Oct 28, 2016 at 1:32 AM, Jakub Sitnicki <jkbs@redhat.com> wrote:
>> On Thu, Oct 27, 2016 at 10:35 PM GMT, Tom Herbert wrote:
>>> On Mon, Oct 24, 2016 at 2:28 AM, Jakub Sitnicki <jkbs@redhat.com> wrote:
>>>> Same as for the transmit path, let's do our best to ensure that received
>>>> ICMP errors that may be subject to forwarding will be routed the same
>>>> path as flow that triggered the error, if it was going in the opposite
>>>> direction.
>>>>
>>> Unfortunately our ability to do this is generally quite limited. This
>>> patch will select the route for multipath, but I don't believe sets
>>> the same link in LAG and definitely can't help switches doing ECMP to
>>> route the ICMP packet in the same way as the flow would be. Did you
>>> see a problem that warrants solving this case?
>>
>> The motivation here is to bring IPv6 ECMP routing on par with IPv4 to
>> enable its wider use, targeting anycast services. Forwarding ICMP errors
>> back to the source host, at the L3 layer, is what we thought would be a
>> step forward.
>>
>> Similar to change in IPv4 routing introduced in commit 79a131592dbb
>> ("ipv4: ICMP packet inspection for multipath", [1]) we do our best at
>> L3, leaving any potential problems with LAG at lower layer (L2)
>> unaddressed.
>>
> ICMP will almost certainly take a different path in the network than
> TCP or UDP due to ECMP. If we ever get proper flow label support for
> ECMP then that could solve the problem if all the devices do a hash
> just on <srcIP, destIP, FlowLabel>.
Sorry for my late reply, I have been traveling.
I think that either I am missing something here, or the proposed changes
address just the problem that you have described.
Yes, if we compute the hash that drives the route choice over the IP
header of the ICMP error, then there is no guarantee it will travel back
to the sender of the offending packet that triggered the error.
That is why, we look at the offending packet carried by an ICMP error
and hash over its fields, instead. We need, however, to take care of two
things:
1) swap the source with the destination address, because we are
forwarding the ICMP error in the opposite direction than the
offending packet was going (see icmpv6_multipath_hash() introduced in
patch 4/5); and
2) ensure the flow labels used in both directions are the same (either
reflected by one side, or fixed, e.g. not used and set to 0), so that
the 4-tuple we hash over when forwarding, <src addr, dst addr, flow
label, next hdr>, is the same both ways, modulo the order of
addresses.
> If this patch is being done to be compatible with IPv4 I guess that's
> okay, but it would be false advertisement to say this makes ICMP
> follow the same path as the flow being targeted in an error.
> Fortunately, I doubt anyone can have a dependency on this for ICMP.
I wouldn't want to propose anything that would be useless. If you think
that this is the case here, I would very much like to understand what
and why cannot work in practice.
Thanks for reviewing this series,
Jakub
^ permalink raw reply
* Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower
From: Jes Sorensen @ 2016-10-30 12:00 UTC (permalink / raw)
To: John Heenan
Cc: Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161030102112.GA5789@cube>
John Heenan <john-AMlJbUfTsso@public.gmane.org> writes:
> Code tests show data returned by rtl8xxxu_read8(priv, REG_CR), used to set
> macpower, is never 0xea. It is only ever 0x01 (first time after modprobe)
> using wpa_supplicant and 0x00 thereafter using wpa_supplicant. These results
> occurs with 'Fix for authentication failure' [PATCH 1/2] in place.
>
> Whatever was returned, code tests always showed that at least
> rtl8xxxu_init_queue_reserved_page(priv);
> is always required. Not called if macpower set to true.
>
> Please see cover letter, [PATCH 0/2], for more information from tests.
>
> For rtl8xxxu-devel branch of git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git
>
> Signed-off-by: John Heenan <john-AMlJbUfTsso@public.gmane.org>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index f25b4df..aae05f3 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -3904,6 +3904,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> macpower = false;
> else
> macpower = true;
> + macpower = false; // Code testing shows macpower must always be set to false to avoid failure
>
> ret = fops->power_on(priv);
> if (ret < 0) {
Sorry but this patch is neither serious nor acceptable. First of all,
hardcoding macpower like this right after an if statement is plain
wrong, second your comments violate all kernel rules.
Second, you argue this was tested using code test - on which device? Did
you test it on all rtl8xxxu based devices or just rtl8723bu?
NACK
Jes
^ permalink raw reply
* [PATCH net-next 4/4] bpf: Add samples for LWT-BPF
From: Thomas Graf @ 2016-10-30 11:58 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, daniel, tom, netdev, roopa
In-Reply-To: <cover.1477827877.git.tgraf@suug.ch>
This adds a set of samples demonstrating the use of lwt-bpf combined
with a shell script which allows running the samples in the form of
a basic selftest.
The samples include:
- Allowing all packets
- Dropping all packets
- Printing context information
- Access packet data
- IPv4 daddr rewrite in dst_output()
- L2 MAC header push + redirect in lwt xmit
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
samples/bpf/bpf_helpers.h | 4 +
samples/bpf/lwt_bpf.c | 210 +++++++++++++++++++++++++++
samples/bpf/test_lwt_bpf.sh | 337 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 551 insertions(+)
create mode 100644 samples/bpf/lwt_bpf.c
create mode 100755 samples/bpf/test_lwt_bpf.sh
diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
index 90f44bd..f34e417 100644
--- a/samples/bpf/bpf_helpers.h
+++ b/samples/bpf/bpf_helpers.h
@@ -80,6 +80,8 @@ struct bpf_map_def {
unsigned int map_flags;
};
+static int (*bpf_skb_load_bytes)(void *ctx, int off, void *to, int len) =
+ (void *) BPF_FUNC_skb_load_bytes;
static int (*bpf_skb_store_bytes)(void *ctx, int off, void *from, int len, int flags) =
(void *) BPF_FUNC_skb_store_bytes;
static int (*bpf_l3_csum_replace)(void *ctx, int off, int from, int to, int flags) =
@@ -88,6 +90,8 @@ static int (*bpf_l4_csum_replace)(void *ctx, int off, int from, int to, int flag
(void *) BPF_FUNC_l4_csum_replace;
static int (*bpf_skb_under_cgroup)(void *ctx, void *map, int index) =
(void *) BPF_FUNC_skb_under_cgroup;
+static int (*bpf_skb_push)(void *, int len, int flags) =
+ (void *) BPF_FUNC_skb_push;
#if defined(__x86_64__)
diff --git a/samples/bpf/lwt_bpf.c b/samples/bpf/lwt_bpf.c
new file mode 100644
index 0000000..05be6ac
--- /dev/null
+++ b/samples/bpf/lwt_bpf.c
@@ -0,0 +1,210 @@
+/* Copyright (c) 2016 Thomas Graf <tgraf@tgraf.ch>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include <stdint.h>
+#include <stddef.h>
+#include <linux/bpf.h>
+#include <linux/ip.h>
+#include <linux/in.h>
+#include <linux/in6.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
+#include <linux/icmpv6.h>
+#include <linux/if_ether.h>
+#include "bpf_helpers.h"
+#include <string.h>
+
+# define printk(fmt, ...) \
+ ({ \
+ char ____fmt[] = fmt; \
+ bpf_trace_printk(____fmt, sizeof(____fmt), \
+ ##__VA_ARGS__); \
+ })
+
+#define CB_MAGIC 1234
+
+/* Let all packets pass */
+SEC("nop")
+int do_nop(struct __sk_buff *skb)
+{
+ return BPF_OK;
+}
+
+/* Print some context information per packet to tracing buffer.
+ */
+SEC("ctx_test")
+int do_ctx_test(struct __sk_buff *skb)
+{
+ skb->cb[0] = CB_MAGIC;
+ printk("len %d hash %d protocol %d\n", skb->len, skb->hash,
+ skb->protocol);
+ printk("cb %d ingress_ifindex %d ifindex %d\n", skb->cb[0],
+ skb->ingress_ifindex, skb->ifindex);
+
+ return BPF_OK;
+}
+
+/* Print content of skb->cb[] to tracing buffer */
+SEC("print_cb")
+int do_print_cb(struct __sk_buff *skb)
+{
+ printk("cb0: %x cb1: %x cb2: %x\n", skb->cb[0], skb->cb[1],
+ skb->cb[2]);
+ printk("cb3: %x cb4: %x\n", skb->cb[3], skb->cb[4]);
+
+ return BPF_OK;
+}
+
+/* Print source and destination IPv4 address to tracing buffer */
+SEC("data_test")
+int do_data_test(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ struct iphdr *iph = data;
+
+ if (data + sizeof(*iph) > data_end) {
+ printk("packet truncated\n");
+ return BPF_DROP;
+ }
+
+ printk("src: %x dst: %x\n", iph->saddr, iph->daddr);
+
+ return BPF_OK;
+}
+
+#define IP_CSUM_OFF offsetof(struct iphdr, check)
+#define IP_DST_OFF offsetof(struct iphdr, daddr)
+#define IP_SRC_OFF offsetof(struct iphdr, saddr)
+#define IP_PROTO_OFF offsetof(struct iphdr, protocol)
+#define TCP_CSUM_OFF offsetof(struct tcphdr, check)
+#define UDP_CSUM_OFF offsetof(struct udphdr, check)
+#define IS_PSEUDO 0x10
+
+static inline int rewrite(struct __sk_buff *skb, uint32_t old_ip,
+ uint32_t new_ip, int rw_daddr)
+{
+ int ret, off = 0, flags = IS_PSEUDO;
+ uint8_t proto;
+
+ ret = bpf_skb_load_bytes(skb, IP_PROTO_OFF, &proto, 1);
+ if (ret < 0) {
+ printk("bpf_l4_csum_replace failed: %d\n", ret);
+ return BPF_DROP;
+ }
+
+ switch (proto) {
+ case IPPROTO_TCP:
+ off = TCP_CSUM_OFF;
+ break;
+
+ case IPPROTO_UDP:
+ off = UDP_CSUM_OFF;
+ flags |= BPF_F_MARK_MANGLED_0;
+ break;
+
+ case IPPROTO_ICMPV6:
+ off = offsetof(struct icmp6hdr, icmp6_cksum);
+ break;
+ }
+
+ if (off) {
+ ret = bpf_l4_csum_replace(skb, off, old_ip, new_ip,
+ flags | sizeof(new_ip));
+ if (ret < 0) {
+ printk("bpf_l4_csum_replace failed: %d\n");
+ return BPF_DROP;
+ }
+ }
+
+ ret = bpf_l3_csum_replace(skb, IP_CSUM_OFF, old_ip, new_ip, sizeof(new_ip));
+ if (ret < 0) {
+ printk("bpf_l3_csum_replace failed: %d\n", ret);
+ return BPF_DROP;
+ }
+
+ if (rw_daddr)
+ ret = bpf_skb_store_bytes(skb, IP_DST_OFF, &new_ip, sizeof(new_ip), 0);
+ else
+ ret = bpf_skb_store_bytes(skb, IP_SRC_OFF, &new_ip, sizeof(new_ip), 0);
+
+ if (ret < 0) {
+ printk("bpf_skb_store_bytes() failed: %d\n", ret);
+ return BPF_DROP;
+ }
+
+ return BPF_OK;
+}
+
+/* Rewrite IPv4 destination address from 192.168.254.2 to 192.168.254.3 */
+SEC("rw_out")
+int do_rw_out(struct __sk_buff *skb)
+{
+ uint32_t old_ip, new_ip = 0x3fea8c0;
+ int ret;
+
+ ret = bpf_skb_load_bytes(skb, IP_DST_OFF, &old_ip, 4);
+ if (ret < 0) {
+ printk("bpf_skb_load_bytes failed: %d\n", ret);
+ return BPF_DROP;
+ }
+
+ if (old_ip == 0x2fea8c0) {
+ printk("out: rewriting from %x to %x\n", old_ip, new_ip);
+ return rewrite(skb, old_ip, new_ip, 1);
+ }
+
+ return BPF_OK;
+}
+
+SEC("redirect")
+int do_redirect(struct __sk_buff *skb)
+{
+ uint64_t smac = SRC_MAC, dmac = DST_MAC;
+ int ret, ifindex = DST_IFINDEX;
+ struct ethhdr ehdr;
+
+ ret = bpf_skb_push(skb, 14, 0);
+ if (ret < 0) {
+ printk("skb_push() failed: %d\n", ret);
+ }
+
+ ehdr.h_proto = __constant_htons(ETH_P_IP);
+ memcpy(&ehdr.h_source, &smac, 6);
+ memcpy(&ehdr.h_dest, &dmac, 6);
+
+ ret = bpf_skb_store_bytes(skb, 0, &ehdr, sizeof(ehdr), 0);
+ if (ret < 0) {
+ printk("skb_store_bytes() failed: %d\n", ret);
+ return BPF_DROP;
+ }
+
+ ret = bpf_redirect(ifindex, 0);
+ if (ret < 0) {
+ printk("bpf_redirect() failed: %d\n", ret);
+ return BPF_DROP;
+ }
+
+ printk("redirected to %d\n", ifindex);
+
+ return BPF_REDIRECT;
+}
+
+/* Drop all packets */
+SEC("drop_all")
+int do_drop_all(struct __sk_buff *skb)
+{
+ printk("dropping with: %d\n", BPF_DROP);
+ return BPF_DROP;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/test_lwt_bpf.sh b/samples/bpf/test_lwt_bpf.sh
new file mode 100755
index 0000000..ea4921a
--- /dev/null
+++ b/samples/bpf/test_lwt_bpf.sh
@@ -0,0 +1,337 @@
+#!/bin/bash
+
+# Uncomment to see generated bytecode
+#VERBOSE=verbose
+
+NS=lwt_ns
+VETH0=tst_lwt0
+VETH1=tst_lwt1
+IP4_1="192.168.254.1"
+IP4_2="192.168.254.2"
+IP4_3="192.168.254.3"
+IP4_4="192.168.254.4"
+
+TRACE_ROOT=/sys/kernel/debug/tracing
+
+function hton_mac()
+{
+ MAC="${1//:/}"
+ echo "0x${MAC:10:2}${MAC:8:2}${MAC:6:2}${MAC:4:2}${MAC:2:2}${MAC:0:2}"
+}
+
+function lookup_mac()
+{
+ set +x
+ if [ ! -z "$2" ]; then
+ MAC=$(ip netns exec $2 ip link show $1 | grep ether | awk '{print $2}')
+ else
+ MAC=$(ip link show $1 | grep ether | awk '{print $2}')
+ fi
+ echo $(hton_mac $MAC)
+ set -x
+}
+
+function cleanup {
+ set +ex
+ rm lwt_bpf.o 2> /dev/null
+ ip link del $VETH0 2> /dev/null
+ ip netns delete $NS 2> /dev/null
+ set -ex
+}
+
+function setup_veth {
+ ip netns add $NS
+
+ ip link add $VETH0 type veth peer name $VETH1
+
+ ip link set dev $VETH0 up
+ ip addr add ${IP4_1}/24 dev $VETH0
+
+ ip link set $VETH1 netns $NS
+ ip netns exec $NS ip link set dev $VETH1 up
+ ip netns exec $NS ip addr add ${IP4_2}/24 dev $VETH1
+ ip netns exec $NS ip addr add ${IP4_3}/32 dev $VETH1
+
+ echo 1 > ${TRACE_ROOT}/tracing_on
+}
+
+function get_trace {
+ set +x
+ cat ${TRACE_ROOT}/trace | grep -v '^#'
+ set -x
+}
+
+function install_prog {
+ ip route del ${IP4_2}/32 dev $VETH0 2> /dev/null || true
+ ip route del table local local ${IP4_4}/32 dev lo 2> /dev/null || true
+ cp /dev/null ${TRACE_ROOT}/trace
+
+ OPTS="encap bpf $1 obj lwt_bpf.o section $2 $VERBOSE"
+
+ if [ "$1" == "in" ]; then
+ ip route add table local local ${IP4_4}/32 $OPTS dev lo
+ else
+ ip route add ${IP4_2}/32 $OPTS dev $VETH0
+ fi
+}
+
+function remove_prog {
+ if [ "$1" == "in" ]; then
+ ip route del table local local ${IP4_4}/32 dev lo
+ else
+ ip route del ${IP4_2}/32 dev $VETH0
+ fi
+}
+
+function filter_trace {
+ # Add newline to allow starting EXPECT= variables on newline
+ NL=$'\n'
+ echo "${NL}$*" | sed -e 's/^.*: : //g'
+}
+
+function expect_fail {
+ set +x
+ echo "FAIL:"
+ echo "Expected: $1"
+ echo "Got: $2"
+ set -x
+ exit 1
+}
+
+function match_trace {
+ set +x
+ RET=0
+ TRACE=$1
+ EXPECT=$2
+ GOT="$(filter_trace "$TRACE")"
+
+ [ "$GOT" != "$EXPECT" ] && {
+ expect_fail "$EXPECT" "$GOT"
+ RET=1
+ }
+ set -x
+ return $RET
+}
+
+function test_start {
+ set +x
+ echo "----------------------------------------------------------------"
+ echo "Starting test: $*"
+ echo "----------------------------------------------------------------"
+ set -x
+}
+
+function failure {
+ get_trace
+ echo "FAIL: $*"
+ exit 1
+}
+
+function test_ctx_xmit {
+ test_start "test_ctx on lwt xmit"
+ install_prog xmit ctx_test
+ ping -c 3 $IP4_2 || {
+ failure "test_ctx xmit: packets are dropped"
+ }
+ match_trace "$(get_trace)" "
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 0 ifindex $DST_IFINDEX
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 0 ifindex $DST_IFINDEX
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 0 ifindex $DST_IFINDEX" || exit 1
+ remove_prog xmit
+}
+
+function test_ctx_out {
+ test_start "test_ctx on lwt out"
+ install_prog out ctx_test
+ ping -c 3 $IP4_2 || {
+ failure "test_ctx out: packets are dropped"
+ }
+ match_trace "$(get_trace)" "
+len 84 hash 0 protocol 0
+cb 1234 ingress_ifindex 0 ifindex 0
+len 84 hash 0 protocol 0
+cb 1234 ingress_ifindex 0 ifindex 0
+len 84 hash 0 protocol 0
+cb 1234 ingress_ifindex 0 ifindex 0" || exit 1
+ remove_prog out
+}
+
+function test_ctx_in {
+ test_start "test_ctx on lwt in"
+ install_prog in ctx_test
+ ping -c 3 $IP4_4 || {
+ failure "test_ctx out: packets are dropped"
+ }
+ # We will both request & reply packets as the packets will
+ # be from $IP4_4 => $IP4_4
+ match_trace "$(get_trace)" "
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 1 ifindex 1
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 1 ifindex 1
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 1 ifindex 1
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 1 ifindex 1
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 1 ifindex 1
+len 84 hash 0 protocol 8
+cb 1234 ingress_ifindex 1 ifindex 1" || exit 1
+ remove_prog in
+}
+
+function test_data {
+ test_start "test_data on lwt $1"
+ install_prog $1 data_test
+ ping -c 3 $IP4_2 || {
+ failure "test_data ${1}: packets are dropped"
+ }
+ match_trace "$(get_trace)" "
+src: 1fea8c0 dst: 2fea8c0
+src: 1fea8c0 dst: 2fea8c0
+src: 1fea8c0 dst: 2fea8c0" || exit 1
+ remove_prog $1
+}
+
+function test_data_in {
+ test_start "test_data on lwt in"
+ install_prog in data_test
+ ping -c 3 $IP4_4 || {
+ failure "test_data in: packets are dropped"
+ }
+ # We will both request & reply packets as the packets will
+ # be from $IP4_4 => $IP4_4
+ match_trace "$(get_trace)" "
+src: 4fea8c0 dst: 4fea8c0
+src: 4fea8c0 dst: 4fea8c0
+src: 4fea8c0 dst: 4fea8c0
+src: 4fea8c0 dst: 4fea8c0
+src: 4fea8c0 dst: 4fea8c0
+src: 4fea8c0 dst: 4fea8c0" || exit 1
+ remove_prog in
+}
+
+function test_cb {
+ test_start "test_cb on lwt $1"
+ install_prog $1 print_cb
+ ping -c 3 $IP4_2 || {
+ failure "test_cb ${1}: packets are dropped"
+ }
+ match_trace "$(get_trace)" "
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0" || exit 1
+ remove_prog $1
+}
+
+function test_cb_in {
+ test_start "test_cb on lwt in"
+ install_prog in print_cb
+ ping -c 3 $IP4_4 || {
+ failure "test_cb in: packets are dropped"
+ }
+ # We will both request & reply packets as the packets will
+ # be from $IP4_4 => $IP4_4
+ match_trace "$(get_trace)" "
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0
+cb0: 0 cb1: 0 cb2: 0
+cb3: 0 cb4: 0" || exit 1
+ remove_prog in
+}
+
+function test_drop_all {
+ test_start "test_drop_all on lwt $1"
+ install_prog $1 drop_all
+ ping -c 3 $IP4_2 && {
+ failure "test_drop_all ${1}: Unexpected success of ping"
+ }
+ match_trace "$(get_trace)" "
+dropping with: 2
+dropping with: 2
+dropping with: 2" || exit 1
+ remove_prog $1
+}
+
+function test_drop_all_in {
+ test_start "test_drop_all on lwt in"
+ install_prog in drop_all
+ ping -c 3 $IP4_4 && {
+ failure "test_drop_all in: Unexpected success of ping"
+ }
+ match_trace "$(get_trace)" "
+dropping with: 2
+dropping with: 2
+dropping with: 2" || exit 1
+ remove_prog in
+}
+
+function test_redirect_xmit {
+ test_start "test_redirect on lwt xmit"
+ install_prog xmit redirect
+ ping -c 3 $IP4_2 || {
+ failure "Redirected packets appear to be dropped"
+ }
+ match_trace "$(get_trace)" "
+redirected to $DST_IFINDEX
+redirected to $DST_IFINDEX
+redirected to $DST_IFINDEX" || exit 1
+ remove_prog xmit
+}
+
+function test_rw_out {
+ test_start "test_rw on lwt out"
+ install_prog out rw_out
+ ping -c 3 $IP4_2 || {
+ failure "FAIL: Redirected packets appear to be dropped"
+ }
+ match_trace "$(get_trace)" "
+redirected to $DST_IFINDEX
+redirected to $DST_IFINDEX
+redirected to $DST_IFINDEX" || exit 1
+ remove_prog out
+}
+
+cleanup
+setup_veth
+
+DST_MAC=$(lookup_mac $VETH1 $NS)
+SRC_MAC=$(lookup_mac $VETH0)
+DST_IFINDEX=$(cat /sys/class/net/$VETH0/ifindex)
+
+CLANG_OPTS="-O2 -target bpf -I ../include/"
+CLANG_OPTS+=" -DSRC_MAC=$SRC_MAC -DDST_MAC=$DST_MAC -DDST_IFINDEX=$DST_IFINDEX"
+clang $CLANG_OPTS -c lwt_bpf.c -o lwt_bpf.o
+
+test_ctx_xmit
+test_ctx_out
+test_ctx_in
+test_data "xmit"
+test_data "out"
+test_data_in
+test_cb "xmit"
+test_cb "out"
+test_cb_in
+test_drop_all "xmit"
+test_drop_all "out"
+test_drop_all_in
+test_redirect_xmit
+test_rw_out
+
+cleanup
+echo 0 > ${TRACE_ROOT}/tracing_on
+exit 0
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 3/4] bpf: BPF for lightweight tunnel encapsulation
From: Thomas Graf @ 2016-10-30 11:58 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, daniel, tom, netdev, roopa
In-Reply-To: <cover.1477827877.git.tgraf@suug.ch>
Register two new BPF prog types BPF_PROG_TYPE_LWT_IN and
BPF_PROG_TYPE_LWT_OUT which are invoked if a route contains a
LWT redirection of type LWTUNNEL_ENCAP_BPF.
The separate program types are required because manipulation of
packet data is only allowed on the output and transmit path as
the subsequent dst_input() call path assumes an IP header
validated by ip_rcv(). The BPF programs will be handed an skb
with the L3 header attached and may return one of the following
return codes:
BPF_OK - Continue routing as per nexthop
BPF_DROP - Drop skb and return EPERM
BPF_REDIRECT - Redirect skb to device as per redirect() helper.
(Only valid on lwtunnel_xmit() hook)
The return codes are binary compatible with their TC_ACT_
relatives to ease compatibility.
A new helper bpf_skb_push() is added which allows to preprend an
L2 header in front of the skb, extend the existing L3 header, or
both. This allows to address a wide range of issues:
- Optimize L2 header construction when L2 information is always
static to avoid ARP/NDisc lookup.
- Extend IP header to add additional IP options.
- Perform simple encapsulation where offload is of no concern.
(The existing funtionality to attach a tunnel key to the skb
and redirect to a tunnel net_device to allow for offload
continues to work obviously).
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
include/linux/filter.h | 2 +-
include/uapi/linux/bpf.h | 31 +++-
include/uapi/linux/lwtunnel.h | 21 +++
kernel/bpf/verifier.c | 16 +-
net/core/Makefile | 2 +-
net/core/filter.c | 148 ++++++++++++++++-
net/core/lwt_bpf.c | 365 ++++++++++++++++++++++++++++++++++++++++++
net/core/lwtunnel.c | 1 +
8 files changed, 579 insertions(+), 7 deletions(-)
create mode 100644 net/core/lwt_bpf.c
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 1f09c52..aad7f81 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -438,7 +438,7 @@ struct xdp_buff {
};
/* compute the linear packet data range [data, data_end) which
- * will be accessed by cls_bpf and act_bpf programs
+ * will be accessed by cls_bpf, act_bpf and lwt programs
*/
static inline void bpf_compute_data_end(struct sk_buff *skb)
{
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e2f38e0..2ebaa3c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -96,6 +96,9 @@ enum bpf_prog_type {
BPF_PROG_TYPE_TRACEPOINT,
BPF_PROG_TYPE_XDP,
BPF_PROG_TYPE_PERF_EVENT,
+ BPF_PROG_TYPE_LWT_IN,
+ BPF_PROG_TYPE_LWT_OUT,
+ BPF_PROG_TYPE_LWT_XMIT,
};
#define BPF_PSEUDO_MAP_FD 1
@@ -383,6 +386,16 @@ union bpf_attr {
*
* int bpf_get_numa_node_id()
* Return: Id of current NUMA node.
+ *
+ * int bpf_skb_push()
+ * Add room to beginning of skb and adjusts MAC header offset accordingly.
+ * Extends/reallocaes for needed skb headeroom automatically.
+ * May change skb data pointer and will thus invalidate any check done
+ * for direct packet access.
+ * @skb: pointer to skb
+ * @len: length of header to be pushed in front
+ * @flags: Flags (unused for now)
+ * Return: 0 on success or negative error
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -427,7 +440,8 @@ union bpf_attr {
FN(skb_pull_data), \
FN(csum_update), \
FN(set_hash_invalid), \
- FN(get_numa_node_id),
+ FN(get_numa_node_id), \
+ FN(skb_push),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -511,6 +525,21 @@ struct bpf_tunnel_key {
__u32 tunnel_label;
};
+/* Generic BPF return codes which all BPF program types may support.
+ * The values are binary compatible with their TC_ACT_* counter-part to
+ * provide backwards compatibility with existing SCHED_CLS and SCHED_ACT
+ * programs.
+ *
+ * XDP is handled seprately, see XDP_*.
+ */
+enum bpf_ret_code {
+ BPF_OK = 0,
+ /* 1 reserved */
+ BPF_DROP = 2,
+ /* 3-6 reserved */
+ BPF_REDIRECT = 7,
+};
+
/* User return codes for XDP prog type.
* A valid XDP program must return one of these defined values. All other
* return codes are reserved for future use. Unknown return codes will result
diff --git a/include/uapi/linux/lwtunnel.h b/include/uapi/linux/lwtunnel.h
index a478fe8..9354d997 100644
--- a/include/uapi/linux/lwtunnel.h
+++ b/include/uapi/linux/lwtunnel.h
@@ -9,6 +9,7 @@ enum lwtunnel_encap_types {
LWTUNNEL_ENCAP_IP,
LWTUNNEL_ENCAP_ILA,
LWTUNNEL_ENCAP_IP6,
+ LWTUNNEL_ENCAP_BPF,
__LWTUNNEL_ENCAP_MAX,
};
@@ -42,4 +43,24 @@ enum lwtunnel_ip6_t {
#define LWTUNNEL_IP6_MAX (__LWTUNNEL_IP6_MAX - 1)
+enum {
+ LWT_BPF_PROG_UNSPEC,
+ LWT_BPF_PROG_FD,
+ LWT_BPF_PROG_NAME,
+ __LWT_BPF_PROG_MAX,
+};
+
+#define LWT_BPF_PROG_MAX (__LWT_BPF_PROG_MAX - 1)
+
+enum {
+ LWT_BPF_UNSPEC,
+ LWT_BPF_IN,
+ LWT_BPF_OUT,
+ LWT_BPF_XMIT,
+ __LWT_BPF_MAX,
+};
+
+#define LWT_BPF_MAX (__LWT_BPF_MAX - 1)
+
+
#endif /* _UAPI_LWTUNNEL_H_ */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9002575..519b58e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -633,12 +633,21 @@ static int check_map_access(struct bpf_verifier_env *env, u32 regno, int off,
#define MAX_PACKET_OFF 0xffff
static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
- const struct bpf_call_arg_meta *meta)
+ const struct bpf_call_arg_meta *meta,
+ enum bpf_access_type t)
{
switch (env->prog->type) {
+ case BPF_PROG_TYPE_LWT_IN:
+ /* dst_input() can't write for now, orig_input may depend on
+ * IP header parsed by ip_rcv().
+ */
+ if (t == BPF_WRITE)
+ return false;
case BPF_PROG_TYPE_SCHED_CLS:
case BPF_PROG_TYPE_SCHED_ACT:
case BPF_PROG_TYPE_XDP:
+ case BPF_PROG_TYPE_LWT_OUT:
+ case BPF_PROG_TYPE_LWT_XMIT:
if (meta)
return meta->pkt_access;
@@ -837,7 +846,7 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off,
err = check_stack_read(state, off, size, value_regno);
}
} else if (state->regs[regno].type == PTR_TO_PACKET) {
- if (t == BPF_WRITE && !may_access_direct_pkt_data(env, NULL)) {
+ if (t == BPF_WRITE && !may_access_direct_pkt_data(env, NULL, t)) {
verbose("cannot write into packet\n");
return -EACCES;
}
@@ -970,7 +979,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
return 0;
}
- if (type == PTR_TO_PACKET && !may_access_direct_pkt_data(env, meta)) {
+ if (type == PTR_TO_PACKET &&
+ !may_access_direct_pkt_data(env, meta, BPF_READ)) {
verbose("helper access to the packet is not allowed\n");
return -EACCES;
}
diff --git a/net/core/Makefile b/net/core/Makefile
index d6508c2..a675fd3 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -23,7 +23,7 @@ obj-$(CONFIG_NETWORK_PHY_TIMESTAMPING) += timestamping.o
obj-$(CONFIG_NET_PTP_CLASSIFY) += ptp_classifier.o
obj-$(CONFIG_CGROUP_NET_PRIO) += netprio_cgroup.o
obj-$(CONFIG_CGROUP_NET_CLASSID) += netclassid_cgroup.o
-obj-$(CONFIG_LWTUNNEL) += lwtunnel.o
+obj-$(CONFIG_LWTUNNEL) += lwtunnel.o lwt_bpf.o
obj-$(CONFIG_DST_CACHE) += dst_cache.o
obj-$(CONFIG_HWBM) += hwbm.o
obj-$(CONFIG_NET_DEVLINK) += devlink.o
diff --git a/net/core/filter.c b/net/core/filter.c
index cd9e2ba..325a9d8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2138,6 +2138,43 @@ static const struct bpf_func_proto bpf_skb_change_tail_proto = {
.arg3_type = ARG_ANYTHING,
};
+BPF_CALL_3(bpf_skb_push, struct sk_buff *, skb, __u32, len, u64, flags)
+{
+ u32 new_len = skb->len + len;
+
+ /* restrict max skb size and check for overflow */
+ if (new_len > __bpf_skb_max_len(skb) || new_len < skb->len)
+ return -ERANGE;
+
+ if (flags)
+ return -EINVAL;
+
+ if (len > 0) {
+ int ret;
+
+ ret = skb_cow(skb, len);
+ if (unlikely(ret < 0))
+ return ret;
+
+ __skb_push(skb, len);
+ memset(skb->data, 0, len);
+ }
+
+ skb_reset_mac_header(skb);
+
+ bpf_compute_data_end(skb);
+ return 0;
+}
+
+static const struct bpf_func_proto bpf_skb_push_proto = {
+ .func = bpf_skb_push,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+};
+
bool bpf_helper_changes_skb_data(void *func)
{
if (func == bpf_skb_vlan_push ||
@@ -2147,7 +2184,8 @@ bool bpf_helper_changes_skb_data(void *func)
func == bpf_skb_change_tail ||
func == bpf_skb_pull_data ||
func == bpf_l3_csum_replace ||
- func == bpf_l4_csum_replace)
+ func == bpf_l4_csum_replace ||
+ func == bpf_skb_push)
return true;
return false;
@@ -2578,6 +2616,75 @@ xdp_func_proto(enum bpf_func_id func_id)
}
}
+static const struct bpf_func_proto *
+lwt_in_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ case BPF_FUNC_skb_load_bytes:
+ return &bpf_skb_load_bytes_proto;
+ case BPF_FUNC_skb_pull_data:
+ return &bpf_skb_pull_data_proto;
+ case BPF_FUNC_csum_diff:
+ return &bpf_csum_diff_proto;
+ case BPF_FUNC_get_cgroup_classid:
+ return &bpf_get_cgroup_classid_proto;
+ case BPF_FUNC_get_route_realm:
+ return &bpf_get_route_realm_proto;
+ case BPF_FUNC_get_hash_recalc:
+ return &bpf_get_hash_recalc_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
+ case BPF_FUNC_get_smp_processor_id:
+ return &bpf_get_smp_processor_id_proto;
+ case BPF_FUNC_skb_under_cgroup:
+ return &bpf_skb_under_cgroup_proto;
+ default:
+ return sk_filter_func_proto(func_id);
+ }
+}
+
+static const struct bpf_func_proto *
+lwt_out_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ case BPF_FUNC_skb_store_bytes:
+ return &bpf_skb_store_bytes_proto;
+ case BPF_FUNC_csum_update:
+ return &bpf_csum_update_proto;
+ case BPF_FUNC_l3_csum_replace:
+ return &bpf_l3_csum_replace_proto;
+ case BPF_FUNC_l4_csum_replace:
+ return &bpf_l4_csum_replace_proto;
+ case BPF_FUNC_set_hash_invalid:
+ return &bpf_set_hash_invalid_proto;
+ default:
+ return lwt_in_func_proto(func_id);
+ }
+}
+
+static const struct bpf_func_proto *
+lwt_xmit_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ case BPF_FUNC_skb_get_tunnel_key:
+ return &bpf_skb_get_tunnel_key_proto;
+ case BPF_FUNC_skb_set_tunnel_key:
+ return bpf_get_skb_set_tunnel_proto(func_id);
+ case BPF_FUNC_skb_get_tunnel_opt:
+ return &bpf_skb_get_tunnel_opt_proto;
+ case BPF_FUNC_skb_set_tunnel_opt:
+ return bpf_get_skb_set_tunnel_proto(func_id);
+ case BPF_FUNC_redirect:
+ return &bpf_redirect_proto;
+ case BPF_FUNC_skb_change_tail:
+ return &bpf_skb_change_tail_proto;
+ case BPF_FUNC_skb_push:
+ return &bpf_skb_push_proto;
+ default:
+ return lwt_out_func_proto(func_id);
+ }
+}
+
static bool __is_valid_access(int off, int size, enum bpf_access_type type)
{
if (off < 0 || off >= sizeof(struct __sk_buff))
@@ -2940,6 +3047,27 @@ static const struct bpf_verifier_ops xdp_ops = {
.convert_ctx_access = xdp_convert_ctx_access,
};
+static const struct bpf_verifier_ops lwt_in_ops = {
+ .get_func_proto = lwt_in_func_proto,
+ .is_valid_access = tc_cls_act_is_valid_access,
+ .convert_ctx_access = sk_filter_convert_ctx_access,
+ .gen_prologue = tc_cls_act_prologue,
+};
+
+static const struct bpf_verifier_ops lwt_out_ops = {
+ .get_func_proto = lwt_out_func_proto,
+ .is_valid_access = tc_cls_act_is_valid_access,
+ .convert_ctx_access = sk_filter_convert_ctx_access,
+ .gen_prologue = tc_cls_act_prologue,
+};
+
+static const struct bpf_verifier_ops lwt_xmit_ops = {
+ .get_func_proto = lwt_xmit_func_proto,
+ .is_valid_access = tc_cls_act_is_valid_access,
+ .convert_ctx_access = sk_filter_convert_ctx_access,
+ .gen_prologue = tc_cls_act_prologue,
+};
+
static struct bpf_prog_type_list sk_filter_type __read_mostly = {
.ops = &sk_filter_ops,
.type = BPF_PROG_TYPE_SOCKET_FILTER,
@@ -2960,12 +3088,30 @@ static struct bpf_prog_type_list xdp_type __read_mostly = {
.type = BPF_PROG_TYPE_XDP,
};
+static struct bpf_prog_type_list lwt_in_type __read_mostly = {
+ .ops = &lwt_in_ops,
+ .type = BPF_PROG_TYPE_LWT_IN,
+};
+
+static struct bpf_prog_type_list lwt_out_type __read_mostly = {
+ .ops = &lwt_out_ops,
+ .type = BPF_PROG_TYPE_LWT_OUT,
+};
+
+static struct bpf_prog_type_list lwt_xmit_type __read_mostly = {
+ .ops = &lwt_xmit_ops,
+ .type = BPF_PROG_TYPE_LWT_XMIT,
+};
+
static int __init register_sk_filter_ops(void)
{
bpf_register_prog_type(&sk_filter_type);
bpf_register_prog_type(&sched_cls_type);
bpf_register_prog_type(&sched_act_type);
bpf_register_prog_type(&xdp_type);
+ bpf_register_prog_type(&lwt_in_type);
+ bpf_register_prog_type(&lwt_out_type);
+ bpf_register_prog_type(&lwt_xmit_type);
return 0;
}
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
new file mode 100644
index 0000000..8404ac6
--- /dev/null
+++ b/net/core/lwt_bpf.c
@@ -0,0 +1,365 @@
+/* Copyright (c) 2016 Thomas Graf <tgraf@tgraf.ch>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/types.h>
+#include <linux/bpf.h>
+#include <net/lwtunnel.h>
+
+struct bpf_lwt_prog {
+ struct bpf_prog *prog;
+ char *name;
+};
+
+struct bpf_lwt {
+ struct bpf_lwt_prog in;
+ struct bpf_lwt_prog out;
+ struct bpf_lwt_prog xmit;
+};
+
+#define MAX_PROG_NAME 256
+
+static inline struct bpf_lwt *bpf_lwt_lwtunnel(struct lwtunnel_state *lwt)
+{
+ return (struct bpf_lwt *)lwt->data;
+}
+
+#define NO_REDIRECT false
+#define CAN_REDIRECT true
+
+static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
+ struct dst_entry *dst, bool can_redirect)
+{
+ int ret;
+
+ /* Preempt disable is needed to protect per-cpu redirect_info between
+ * BPF prog and skb_do_redirect(). The call_rcu in bpf_prog_put() and
+ * access to maps strictly require a rcu_read_lock() for protection,
+ * mixing with BH RCU lock doesn't work.
+ */
+ preempt_disable();
+ rcu_read_lock();
+ bpf_compute_data_end(skb);
+ ret = BPF_PROG_RUN(lwt->prog, skb);
+ rcu_read_unlock();
+
+ switch (ret) {
+ case BPF_OK:
+ break;
+
+ case BPF_REDIRECT:
+ if (!can_redirect) {
+ WARN_ONCE(1, "Illegal redirect return code in prog %s\n",
+ lwt->name ? : "<unknown>");
+ ret = BPF_OK;
+ } else {
+ ret = skb_do_redirect(skb);
+ if (ret == 0)
+ ret = BPF_REDIRECT;
+ }
+ break;
+
+ case BPF_DROP:
+ kfree_skb(skb);
+ ret = -EPERM;
+ break;
+
+ default:
+ WARN_ONCE(1, "Illegal LWT BPF return value %u, expect packet loss\n",
+ ret);
+ kfree_skb(skb);
+ ret = -EINVAL;
+ break;
+ }
+
+ preempt_enable();
+
+ return ret;
+}
+
+static int bpf_input(struct sk_buff *skb)
+{
+ struct dst_entry *dst = skb_dst(skb);
+ struct bpf_lwt *bpf;
+ int ret;
+
+ bpf = bpf_lwt_lwtunnel(dst->lwtstate);
+ if (bpf->in.prog) {
+ ret = run_lwt_bpf(skb, &bpf->in, dst, NO_REDIRECT);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (unlikely(!dst->lwtstate->orig_input)) {
+ WARN_ONCE(1, "orig_input not set on dst for prog %s\n",
+ bpf->out.name);
+ kfree_skb(skb);
+ return -EINVAL;
+ }
+
+ return dst->lwtstate->orig_input(skb);
+}
+
+static int bpf_output(struct net *net, struct sock *sk, struct sk_buff *skb)
+{
+ struct dst_entry *dst = skb_dst(skb);
+ struct bpf_lwt *bpf;
+ int ret;
+
+ bpf = bpf_lwt_lwtunnel(dst->lwtstate);
+ if (bpf->out.prog) {
+ ret = run_lwt_bpf(skb, &bpf->out, dst, NO_REDIRECT);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (unlikely(!dst->lwtstate->orig_output)) {
+ WARN_ONCE(1, "orig_output not set on dst for prog %s\n",
+ bpf->out.name);
+ kfree_skb(skb);
+ return -EINVAL;
+ }
+
+ return dst->lwtstate->orig_output(net, sk, skb);
+}
+
+static int bpf_xmit(struct sk_buff *skb)
+{
+ struct dst_entry *dst = skb_dst(skb);
+ struct bpf_lwt *bpf;
+
+ bpf = bpf_lwt_lwtunnel(dst->lwtstate);
+ if (bpf->xmit.prog) {
+ int ret;
+
+ ret = run_lwt_bpf(skb, &bpf->xmit, dst, CAN_REDIRECT);
+ switch (ret) {
+ case BPF_OK:
+ return LWTUNNEL_XMIT_CONTINUE;
+ case BPF_REDIRECT:
+ return LWTUNNEL_XMIT_DONE;
+ default:
+ return ret;
+ }
+ }
+
+ return LWTUNNEL_XMIT_CONTINUE;
+}
+
+static void bpf_lwt_prog_destroy(struct bpf_lwt_prog *prog)
+{
+ if (prog->prog)
+ bpf_prog_put(prog->prog);
+
+ kfree(prog->name);
+}
+
+static void bpf_destroy_state(struct lwtunnel_state *lwt)
+{
+ struct bpf_lwt *bpf = bpf_lwt_lwtunnel(lwt);
+
+ bpf_lwt_prog_destroy(&bpf->in);
+ bpf_lwt_prog_destroy(&bpf->out);
+ bpf_lwt_prog_destroy(&bpf->xmit);
+}
+
+static const struct nla_policy bpf_prog_policy[LWT_BPF_PROG_MAX + 1] = {
+ [LWT_BPF_PROG_FD] = { .type = NLA_U32, },
+ [LWT_BPF_PROG_NAME] = { .type = NLA_NUL_STRING,
+ .len = MAX_PROG_NAME },
+};
+
+static int bpf_parse_prog(struct nlattr *attr, struct bpf_lwt_prog *prog,
+ enum bpf_prog_type type)
+{
+ struct nlattr *tb[LWT_BPF_PROG_MAX + 1];
+ struct bpf_prog *p;
+ int ret;
+ u32 fd;
+
+ ret = nla_parse_nested(tb, LWT_BPF_PROG_MAX, attr, bpf_prog_policy);
+ if (ret < 0)
+ return ret;
+
+ if (!tb[LWT_BPF_PROG_FD] || !tb[LWT_BPF_PROG_NAME])
+ return -EINVAL;
+
+ prog->name = nla_memdup(tb[LWT_BPF_PROG_NAME], GFP_KERNEL);
+ if (!prog->name)
+ return -ENOMEM;
+
+ fd = nla_get_u32(tb[LWT_BPF_PROG_FD]);
+ p = bpf_prog_get_type(fd, type);
+ if (IS_ERR(p))
+ return PTR_ERR(p);
+
+ prog->prog = p;
+
+ return 0;
+}
+
+static const struct nla_policy bpf_nl_policy[LWT_BPF_MAX + 1] = {
+ [LWT_BPF_IN] = { .type = NLA_NESTED, },
+ [LWT_BPF_OUT] = { .type = NLA_NESTED, },
+ [LWT_BPF_XMIT] = { .type = NLA_NESTED, },
+};
+
+static int bpf_build_state(struct net_device *dev, struct nlattr *nla,
+ unsigned int family, const void *cfg,
+ struct lwtunnel_state **ts)
+{
+ struct nlattr *tb[LWT_BPF_MAX + 1];
+ struct lwtunnel_state *newts;
+ struct bpf_lwt *bpf;
+ int ret;
+
+ ret = nla_parse_nested(tb, LWT_BPF_MAX, nla, bpf_nl_policy);
+ if (ret < 0)
+ return ret;
+
+ if (!tb[LWT_BPF_IN] && !tb[LWT_BPF_OUT] && !tb[LWT_BPF_XMIT])
+ return -EINVAL;
+
+ newts = lwtunnel_state_alloc(sizeof(*bpf));
+ if (!newts)
+ return -ENOMEM;
+
+ newts->type = LWTUNNEL_ENCAP_BPF;
+ bpf = bpf_lwt_lwtunnel(newts);
+
+ if (tb[LWT_BPF_IN]) {
+ ret = bpf_parse_prog(tb[LWT_BPF_IN], &bpf->in,
+ BPF_PROG_TYPE_LWT_IN);
+ if (ret < 0) {
+ kfree(newts);
+ return ret;
+ }
+
+ newts->flags |= LWTUNNEL_STATE_INPUT_REDIRECT;
+ }
+
+ if (tb[LWT_BPF_OUT]) {
+ ret = bpf_parse_prog(tb[LWT_BPF_OUT], &bpf->out,
+ BPF_PROG_TYPE_LWT_OUT);
+ if (ret < 0) {
+ bpf_destroy_state(newts);
+ kfree(newts);
+ return ret;
+ }
+
+ newts->flags |= LWTUNNEL_STATE_OUTPUT_REDIRECT;
+ }
+
+ if (tb[LWT_BPF_XMIT]) {
+ ret = bpf_parse_prog(tb[LWT_BPF_XMIT], &bpf->xmit,
+ BPF_PROG_TYPE_LWT_XMIT);
+ if (ret < 0) {
+ bpf_destroy_state(newts);
+ kfree(newts);
+ return ret;
+ }
+
+ newts->flags |= LWTUNNEL_STATE_XMIT_REDIRECT;
+ }
+
+ *ts = newts;
+
+ return 0;
+}
+
+static int bpf_fill_lwt_prog(struct sk_buff *skb, int attr,
+ struct bpf_lwt_prog *prog)
+{
+ struct nlattr *nest;
+
+ if (!prog->prog)
+ return 0;
+
+ nest = nla_nest_start(skb, attr);
+ if (!nest)
+ return -EMSGSIZE;
+
+ if (prog->name &&
+ nla_put_string(skb, LWT_BPF_PROG_NAME, prog->name))
+ return -EMSGSIZE;
+
+ return nla_nest_end(skb, nest);
+}
+
+static int bpf_fill_encap_info(struct sk_buff *skb, struct lwtunnel_state *lwt)
+{
+ struct bpf_lwt *bpf = bpf_lwt_lwtunnel(lwt);
+
+ if (bpf_fill_lwt_prog(skb, LWT_BPF_IN, &bpf->in) < 0 ||
+ bpf_fill_lwt_prog(skb, LWT_BPF_OUT, &bpf->out) < 0 ||
+ bpf_fill_lwt_prog(skb, LWT_BPF_XMIT, &bpf->xmit) < 0)
+ return -EMSGSIZE;
+
+ return 0;
+}
+
+static int bpf_encap_nlsize(struct lwtunnel_state *lwtstate)
+{
+ int nest_len = nla_total_size(sizeof(struct nlattr)) +
+ nla_total_size(MAX_PROG_NAME) + /* LWT_BPF_PROG_NAME */
+ 0;
+
+ return nest_len + /* LWT_BPF_IN */
+ nest_len + /* LWT_BPF_OUT */
+ nest_len + /* LWT_BPF_XMIT */
+ 0;
+}
+
+int bpf_lwt_prog_cmp(struct bpf_lwt_prog *a, struct bpf_lwt_prog *b)
+{
+ /* FIXME:
+ * The LWT state is currently rebuilt for delete requests which
+ * results in a new bpf_prog instance. Comparing names for now.
+ */
+ if (!a->name && !b->name)
+ return 0;
+
+ if (!a->name || !b->name)
+ return 1;
+
+ return strcmp(a->name, b->name);
+}
+
+static int bpf_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b)
+{
+ struct bpf_lwt *a_bpf = bpf_lwt_lwtunnel(a);
+ struct bpf_lwt *b_bpf = bpf_lwt_lwtunnel(b);
+
+ return bpf_lwt_prog_cmp(&a_bpf->in, &b_bpf->in) ||
+ bpf_lwt_prog_cmp(&a_bpf->out, &b_bpf->out) ||
+ bpf_lwt_prog_cmp(&a_bpf->xmit, &b_bpf->xmit);
+}
+
+static const struct lwtunnel_encap_ops bpf_encap_ops = {
+ .build_state = bpf_build_state,
+ .destroy_state = bpf_destroy_state,
+ .input = bpf_input,
+ .output = bpf_output,
+ .xmit = bpf_xmit,
+ .fill_encap = bpf_fill_encap_info,
+ .get_encap_size = bpf_encap_nlsize,
+ .cmp_encap = bpf_encap_cmp,
+};
+
+static int __init bpf_lwt_init(void)
+{
+ return lwtunnel_encap_add_ops(&bpf_encap_ops, LWTUNNEL_ENCAP_BPF);
+}
+
+subsys_initcall(bpf_lwt_init)
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index 88fd642..554d901 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -39,6 +39,7 @@ static const char *lwtunnel_encap_str(enum lwtunnel_encap_types encap_type)
return "MPLS";
case LWTUNNEL_ENCAP_ILA:
return "ILA";
+ case LWTUNNEL_ENCAP_BPF:
case LWTUNNEL_ENCAP_IP6:
case LWTUNNEL_ENCAP_IP:
case LWTUNNEL_ENCAP_NONE:
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/4] BPF for lightweight tunnel encapsulation
From: Thomas Graf @ 2016-10-30 11:58 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, daniel, tom, netdev, roopa
This series implements BPF program invocation from dst entries via the
lightweight tunnels infrastructure. The BPF program can be attached to
lwtunnel_input(), lwtunnel_output() or lwtunnel_xmit() and sees an L3
skb as context. input is read-only, output can write, xmit can write,
push headers, and redirect.
Motiviation for this work:
- Restricting outgoing routes beyond what the route tuple supports
- Per route accounting byond realms
- Fast attachment of L2 headers where header does not require resolving
L2 addresses
- ILA like uses cases where L3 addresses are resolved and then routed
in an async manner
- Fast encapsulation + redirect. For now limited to use cases where not
setting inner and outer offset/protocol is OK.
A couple of samples on how to use it can be found in patch 04.
Thomas Graf (4):
route: Set orig_output when redirecting to lwt on locally generated
traffic
route: Set lwtstate for local traffic and cached input dsts
bpf: BPF for lightweight tunnel encapsulation
bpf: Add samples for LWT-BPF
include/linux/filter.h | 2 +-
include/uapi/linux/bpf.h | 31 +++-
include/uapi/linux/lwtunnel.h | 21 +++
kernel/bpf/verifier.c | 16 +-
net/core/Makefile | 2 +-
net/core/filter.c | 148 ++++++++++++++++-
net/core/lwt_bpf.c | 365 ++++++++++++++++++++++++++++++++++++++++++
net/core/lwtunnel.c | 1 +
net/ipv4/route.c | 37 +++--
samples/bpf/bpf_helpers.h | 4 +
samples/bpf/lwt_bpf.c | 210 ++++++++++++++++++++++++
samples/bpf/test_lwt_bpf.sh | 337 ++++++++++++++++++++++++++++++++++++++
12 files changed, 1156 insertions(+), 18 deletions(-)
create mode 100644 net/core/lwt_bpf.c
create mode 100644 samples/bpf/lwt_bpf.c
create mode 100755 samples/bpf/test_lwt_bpf.sh
--
2.7.4
^ permalink raw reply
* [PATCH net-next 2/4] route: Set lwtstate for local traffic and cached input dsts
From: Thomas Graf @ 2016-10-30 11:58 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, daniel, tom, netdev, roopa
In-Reply-To: <cover.1477827877.git.tgraf@suug.ch>
A route on the output path hitting a RTN_LOCAL route will keep the dst
associated on its way through the loopback device. On the receive path,
the dst_input() call will thus invoke the input handler of the route
created in the output path. Thus, lwt redirection for input must be done
for dsts allocated in the otuput path as well.
Also, if a route is cached in the input path, the allocated dst should
respect lwtunnel configuration on the nexthop as well.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/ipv4/route.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 7da886e..44f5403 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1596,6 +1596,19 @@ static void ip_del_fnhe(struct fib_nh *nh, __be32 daddr)
spin_unlock_bh(&fnhe_lock);
}
+static void set_lwt_redirect(struct rtable *rth)
+{
+ if (lwtunnel_output_redirect(rth->dst.lwtstate)) {
+ rth->dst.lwtstate->orig_output = rth->dst.output;
+ rth->dst.output = lwtunnel_output;
+ }
+
+ if (lwtunnel_input_redirect(rth->dst.lwtstate)) {
+ rth->dst.lwtstate->orig_input = rth->dst.input;
+ rth->dst.input = lwtunnel_input;
+ }
+}
+
/* called in rcu_read_lock() section */
static int __mkroute_input(struct sk_buff *skb,
const struct fib_result *res,
@@ -1685,14 +1698,7 @@ static int __mkroute_input(struct sk_buff *skb,
rth->dst.input = ip_forward;
rt_set_nexthop(rth, daddr, res, fnhe, res->fi, res->type, itag);
- if (lwtunnel_output_redirect(rth->dst.lwtstate)) {
- rth->dst.lwtstate->orig_output = rth->dst.output;
- rth->dst.output = lwtunnel_output;
- }
- if (lwtunnel_input_redirect(rth->dst.lwtstate)) {
- rth->dst.lwtstate->orig_input = rth->dst.input;
- rth->dst.input = lwtunnel_input;
- }
+ set_lwt_redirect(rth);
skb_dst_set(skb, &rth->dst);
out:
err = 0;
@@ -1919,8 +1925,18 @@ out: return err;
rth->dst.error= -err;
rth->rt_flags &= ~RTCF_LOCAL;
}
+
if (do_cache) {
- if (unlikely(!rt_cache_route(&FIB_RES_NH(res), rth))) {
+ struct fib_nh *nh = &FIB_RES_NH(res);
+
+ rth->dst.lwtstate = lwtstate_get(nh->nh_lwtstate);
+ if (lwtunnel_input_redirect(rth->dst.lwtstate)) {
+ WARN_ON(rth->dst.input == lwtunnel_input);
+ rth->dst.lwtstate->orig_input = rth->dst.input;
+ rth->dst.input = lwtunnel_input;
+ }
+
+ if (unlikely(!rt_cache_route(nh, rth))) {
rth->dst.flags |= DST_NOCACHE;
rt_add_uncached_list(rth);
}
@@ -2138,10 +2154,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
}
rt_set_nexthop(rth, fl4->daddr, res, fnhe, fi, type, 0);
- if (lwtunnel_output_redirect(rth->dst.lwtstate)) {
- rth->dst.lwtstate->orig_output = rth->dst.output;
- rth->dst.output = lwtunnel_output;
- }
+ set_lwt_redirect(rth);
return rth;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 1/4] route: Set orig_output when redirecting to lwt on locally generated traffic
From: Thomas Graf @ 2016-10-30 11:58 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, daniel, tom, netdev, roopa
In-Reply-To: <cover.1477827877.git.tgraf@suug.ch>
orig_output for IPv4 was only set for dsts which hit an input route.
Set it consistently for locally generated traffic as well to allow
lwt to continue the dst_output() path as configured by the nexthop.
Fixes: 2536862311d ("lwt: Add support to redirect dst.input")
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/ipv4/route.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 62d4d90..7da886e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2138,8 +2138,10 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
}
rt_set_nexthop(rth, fl4->daddr, res, fnhe, fi, type, 0);
- if (lwtunnel_output_redirect(rth->dst.lwtstate))
+ if (lwtunnel_output_redirect(rth->dst.lwtstate)) {
+ rth->dst.lwtstate->orig_output = rth->dst.output;
rth->dst.output = lwtunnel_output;
+ }
return rth;
}
--
2.7.4
^ permalink raw reply related
* Re: [bnx2] [Regression 4.8] Driver loading fails without firmware
From: Paul Menzel @ 2016-10-30 11:05 UTC (permalink / raw)
To: Baoquan He
Cc: Sony Chacko, Dept-HSGLinuxNICDev, netdev, David S. Miller, dvteam
In-Reply-To: <20161029025538.GA4099@x1>
Dear Baoquan,
Am Samstag, den 29.10.2016, 10:55 +0800 schrieb Baoquan He:
> On 10/27/16 at 03:21pm, Paul Menzel wrote:
> > > > Baoquan, could you please fix this regression. My suggestion is, that you
> > > > add the old code back, but check if the firmware has been loaded. If it
> > > > hasn’t, load it again.
> > > >
> > > > That way, people can update their Linux kernel, and it continues working
> > > > without changing the initramfs, or anything else.
> > >
> > > I saw your mail but I am also not familiar with bnx2 driver. As the
> > > commit log says I just tried to make bnx2 driver reset itself earlier.
> > >
> > > So you did a git bisect and found this commit caused the regression,
> > > right? If yes, and network developers have no action, I will look into
> > > the code and see if I have idea to fix it.
> >
> > Well, I looked through the commits and found that one, which would explain
> > the changed behavior.
> >
> > To be sure, and to follow your request, I took Linux 4.8.4 and reverted your
> > commit (attached). Then I deleted the firmware again from the initramfs, and
> > rebooted. The devices showed up just fine as before.
> >
> > So to summarize, the commit is indeed the culprit.
> Sorry for this.
>
> Could you tell the steps to reproduce? I will find a machine with bnx2
> NIC and check if there's other ways.
Well, delete the bnx2 firmware files from the initramfs, and start the
system.
Did you read my proposal, to try to load the firmware twice, that means,
basically revert only the deleted lines of your commit, and add an
additional check?
Kind regards,
Paul
^ permalink raw reply
* RFC if==else in halbtc8723b1ant.c
From: Nicholas Mc Guire @ 2016-10-30 11:03 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, netdev, linux-kernel
Hi !
in your commit f5b586909581 ("rtlwifi: btcoexist: Modify driver to support
BT coexistence in rtl8723be") you introduced a if/else where both branches
are the same but the comment in the else branch suggests that this might be
unintended.
from code review only I can´t say what the intent is.
/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c:halbtc8723b1ant_action_wifi_connected_bt_acl_busy()
1838 if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) ||
1839 (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
1840 halbtc8723b1ant_ps_tdma(btcoexist, NORMAL_EXEC,
1841 true, 14);
1842 coex_dm->auto_tdma_adjust = false;
1843 } else { /*for low BT RSSI*/
1844 halbtc8723b1ant_ps_tdma(btcoexist, NORMAL_EXEC,
1845 true, 14);
1846 coex_dm->auto_tdma_adjust = false;
1847 }
basically the same construct is also in
halbtc8723b1ant_run_coexist_mechanism()
2213 if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
2214 (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
2215 halbtc8723b1ant_limited_tx(btcoexist,
2216 NORMAL_EXEC,
2217 1, 1, 1, 1);
2218 } else {
2219 halbtc8723b1ant_limited_tx(btcoexist,
2220 NORMAL_EXEC,
2221 1, 1, 1, 1);
2222 }
where the if condition is the same so the else may also only apply to the
low BT RSSI - and the if and else are again the same - if this is intended
or not is not clear. If this is intended it should have appropriate comments.
thx!
hofrat
^ permalink raw reply
* Re: Let's do P4
From: Thomas Graf @ 2016-10-30 10:26 UTC (permalink / raw)
To: Jiri Pirko
Cc: John Fastabend, Jakub Kicinski, netdev, davem, jhs, roopa,
simon.horman, ast, daniel, prem, hannes, jbenc, tom, mattyk,
idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
f.fainelli, dsa, vivien.didelot, andrew, ivecera,
Maciej Żenczykowski
In-Reply-To: <20161030074458.GB1686@nanopsycho.orion>
On 10/30/16 at 08:44am, Jiri Pirko wrote:
> Sat, Oct 29, 2016 at 06:46:21PM CEST, john.fastabend@gmail.com wrote:
> >On 16-10-29 07:49 AM, Jakub Kicinski wrote:
> >> On Sat, 29 Oct 2016 09:53:28 +0200, Jiri Pirko wrote:
> >>> Hi all.
> >>>
> >>> The network world is divided into 2 general types of hw:
> >>> 1) network ASICs - network specific silicon, containing things like TCAM
> >>> These ASICs are suitable to be programmed by P4.
> >>> 2) network processors - basically a general purpose CPUs
> >>> These processors are suitable to be programmed by eBPF.
> >>>
> >>> I believe that by now, the most people came to a conclusion that it is
> >>> very difficult to handle both types by either P4 or eBPF. And since
> >>> eBPF is part of the kernel, I would like to introduce P4 into kernel
> >>> as well. Here's a plan:
> >>>
> >>> 1) Define P4 intermediate representation
> >>> I cannot imagine loading P4 program (c-like syntax text file) into
> >>> kernel as is. That means that as the first step, we need find some
> >>> intermediate representation. I can imagine someting in a form of AST,
> >>> call it "p4ast". I don't really know how to do this exactly though,
> >>> it's just an idea.
> >>>
> >>> In the end there would be a userspace precompiler for this:
> >>> $ makep4ast example.p4 example.ast
> >>
> >> Maybe stating the obvious, but IMHO defining the IR is the hardest part.
> >> eBPF *is* the IR, we can compile C, P4 or even JIT Lua to eBPF. The
> >> AST/IR for switch pipelines should allow for similar flexibility.
> >> Looser coupling would also protect us from changes in spec of the high
> >> level language.
My assumption was that a new IR is defined which is easier to parse than
eBPF which is targeted at execution on a CPU and not indented for pattern
matching. Just looking at how llvm creates different patterns and reorders
instructions, I'm not seeing how eBPF can serve as a general purpose IR
if the objective is to allow fairly flexible generation of the bytecode.
Hence the alternative IR serving as additional metadata complementing the
eBPF program.
> >Jumping in the middle here. You managed to get an entire thread going
> >before I even woke up :)
> >
> >The problem with eBPF as an IR is that in the universe of eBPF IR
> >programs the subset that can be offloaded onto a standard ASIC based
> >hardware (non NPU/FPGA/etc) is so small to be almost meaningless IMO.
> >
> >I tried this for awhile and the result is users have to write very
> >targeted eBPF that they "know" will be pattern matched and pushed into
> >an ASIC. It can work but its very fragile. When I did this I ended up
> >with an eBPF generator for deviceX and an eBPF generator for deviceY
> >each with a very specific pattern matching engine in the driver to
> >xlate ebpf-deviceX into its asic. Existing ASICs for example usually
> >support only one pipeline, only one parser (or require moving mountains
> >to change the parse via ucode), only one set of tables, and only one
> >deparser/serailizer at the end to build the new packet. Next-gen pieces
> >may have some flexibility on the parser side.
> >
> >There is an interesting resource allocation problem we have that could
> >be solved by p4 or devlink where in we want to pre-allocate slices of
> >the TCAM for certain match types. I was planning on writing devlink code
> >for this because its primarily done at initialization once.
>
> There are 2 resource allocation problems in our hw. One is general
> division ot the resources in feature-chunks. That needs to be done
> during the ASIC initialization phase. For that, I also plan to utilize
> devlink API.
>
> The second one is runtime allocation of tables, and that would be
> handled by p4 just fine.
>
>
> >
> >I will note one nice thing about using eBPF however is that you have an
> >easy software emulation path via ebpf engine in kernel.
> >
> >... And merging threads here with Jiri's email ...
> >
> >> If you do p4>ebpf in userspace, you have 2 apis:
> >> 1) to setup sw (in-kernel) p4 datapath, you push bpf.o to kernel
> >> 2) to setup hw p4 datapath, you push program.p4ast to kernel
> >>
> >> Those are 2 apis. Both wrapped up by TC, but still 2 apis.
> >>
> >> What I believe is correct is to have one api:
> >> 1) to setup sw (in-kernel) p4 datapath, you push program.p4ast to kernel
> >> 2) to setup hw p4 datapath, you push program.p4ast to kernel
I understand what you mean with two APIs now. You want a single IR
block and divide the SW/HW part in the kernel rather than let llvm or
something else do it.
> >Couple comments around this, first adding yet another IR in the kernel
> >and another JIT engine to map that IR on to eBPF or hardware vendor X
> >doesn't get me excited. Its really much easier to write these as backend
> >objects in LLVM. Not saying it can't be done just saying it is easier
> >in LLVM. Also we already have the LLVM code for P4 to LLVM-IR to eBPF.
> >In the end this would be a reasonably complex bit of code in
> >the kernel only for hardware offload. I have doubts that folks would
> >ever use it for software only cases. I'm happy to admit I'm wrong here
> >though.
>
> Well for hw offload, every driver has to parse the IR (whatever will it
> be in) and program HW accordingly. Similar parsing and translation would
> be needed for SW path, to translate into eBPF. I don't think it would be
> more complex than in the drivers. Should be fine.
I'm not sure I see why anyone would ever want to use an IR for SW
purposes which is restricted to the lowest common denominator of HW.
A good example here is OpenFlow and how some of its SW consumers
have evolved with extensions which cannot be mappepd to HW easily.
The same seems to happen with P4 as it introduces the concept of
state and other concepts which are hard to map for dumb HW. P4 doesn't
magically solve this problem, the fundamental difference in
capabilities between HW and SW remain.
> >So yes using llvm backends creates two paths a hardware mgmt and sw
> >path but in the hardware + software case typical on the edge the
> >orchestration and management planes have started to manage the hardware
> >and software as two blocks of logic for performance SLA logic. Even on
> >the edge it seems in most cases folks are selling SR-IOV ports and
> >can't fall back to software and charge for the port. But this is just
> >one use case I suspect others where it does make sense.
> >
> >> In case of 1), the program.p4ast will be either interpreted by new p4
> >> interpreter, of translated to bpf and interpreted by that. But this
> >> translation code is part of kernel.
> >
> >Finally a couple historic bits. The Flow-API proposed in Ottawa was
> >mechanically generated from an original P4 draft. At the time I was
> >working fairly closely with both the hardware and compiler folks. If
> >there is interest we could use that as a base IR for hardware. It has
> >a simple mapping to/from the original P4 spec. The newer P4 specs are
> >significantly more complex by the way.
>
> Yeah, I was also thinking about something similar to your Flow-API,
> but we need something more generic I believe.
>
> >We also have an emulated path also auto-generated from compiler tools
> >that creates eBPF code from the IR so this would give you the software
> >fall-back.
>
> Btw, Flow-API was rejected because it was a clean kernel-bypass. In case
> of p4, if we do what Thomas is suggesting, having x.bpf for SW and
> x.p4ast for HW, that would be the very same kernel-bypass. Therefore I
> strongly believe there should be a single kernel API for p4 SW+HW - for
> both p4 program insertion and runtime configuration.
I think you misunderstand me. This is not what I'm proposing at all.
In either model, the kernel receives the same IR and can reject.
The rule is very clear: we can't allow to program anything that the
kernel is not capable of doing in SW, right? That was the key take
away from that discussion.
Let's assume we do cls_p4ast HW+SW with an eBPF translator for SW. As a
user of this, as my program becomes more complex I will hit the wall of
HW capabilities at some point and either the IR is not expressive
enough or the driver will reject the program.
I have at least three options now:
1) I move everything to SW and forget about HW programmability
2) I let HW bail out when HW can't support it and start parsing from
scratch in SW. I don't really care how much of it has been done
in HW, it's a best effort optimization. A use case for this might
be dropping of packets. This is easy to do with flow based
offloads as it can be best effort but already difficult when
programming based on a IR.
3) I let HW bail out but carry some metadata trying to preserve
some of the work done. A use case for this would be a router type
of work where the lookup itself can be expensive and the majority
of actions taken on packets are simple forwards but a subset of
actions performed are too complex for HW. You still want to
preserve the savings from the expensive lookup already performed.
Even for the simpler 2) I can't just put everything I need into my
p4ast program because the program will either load in its entirety or
it won't.
What I would likely end up doing is to write any number of subsets of
my program which only contain various levels of pieces that are very
likely to load on my target HW. I then load my full program with tc
and want a notification if it also loaded into HW. If it HW failed,
then I want tc to load subset programs with SKIP_SW starting from the
one with most complexity. I need SKIP_SW because I already have the
full program loaded and I don't want to go through both the partial
and full program in case of a HW bail out. Is your proposal to not
allow for the SKIP_SW?
A more evolved form of this would be to expose capabilities of the HW
and have the program writer include logic which results in the split
based on the capabilities of the hardware.
I I understand you correctly, you propose to make this split
automatically in the kernel somehow.
In either model the kernel receives the same new IR which it can
reject. No difference. None of the models allow more or less.
In either model, the program can be loaded with SKIP_SW to load a valid
program into HW only.
In either model, an eBPF program can be loaded at cls_bpf, or a new IR
can be loaded with SKIP_HW to do SW only.
The only difference I see between the models is whether to include a
new IR => eBPF compiler in the kernel or not which is going to be
optional anyway.
^ permalink raw reply
* [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower
From: John Heenan @ 2016-10-30 10:21 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo, linux-wireless, netdev; +Cc: linux-kernel
In-Reply-To: <cover.1477769750.git.john@zgus.com>
Code tests show data returned by rtl8xxxu_read8(priv, REG_CR), used to set
macpower, is never 0xea. It is only ever 0x01 (first time after modprobe)
using wpa_supplicant and 0x00 thereafter using wpa_supplicant. These results
occurs with 'Fix for authentication failure' [PATCH 1/2] in place.
Whatever was returned, code tests always showed that at least
rtl8xxxu_init_queue_reserved_page(priv);
is always required. Not called if macpower set to true.
Please see cover letter, [PATCH 0/2], for more information from tests.
For rtl8xxxu-devel branch of git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git
Signed-off-by: John Heenan <john@zgus.com>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index f25b4df..aae05f3 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3904,6 +3904,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
macpower = false;
else
macpower = true;
+ macpower = false; // Code testing shows macpower must always be set to false to avoid failure
ret = fops->power_on(priv);
if (ret < 0) {
--
2.10.1
^ permalink raw reply related
* [PATCH 1/2] rtl8xxxu: Fix for authentication failure
From: John Heenan @ 2016-10-30 10:20 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo, linux-wireless, netdev; +Cc: linux-kernel
In-Reply-To: <cover.1477769750.git.john@zgus.com>
This fix enables the same sequence of init behaviour as the alternative
working driver for the wireless rtl8723bu IC at
https://github.com/lwfinger/rtl8723bu
For exampe rtl8xxxu_init_device is now called each time
userspace wpa_supplicant is executed instead of just once when
modprobe is executed.
Along with 'Fix for bogus data used to determine macpower',
wpa_supplicant now reliably and successfully authenticates.
For rtl8xxxu-devel branch of git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git
Signed-off-by: John Heenan <john@zgus.com>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 04141e5..f25b4df 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5779,6 +5779,11 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
ret = 0;
+ ret = rtl8xxxu_init_device(hw);
+ if (ret)
+ goto error_out;
+
+
init_usb_anchor(&priv->rx_anchor);
init_usb_anchor(&priv->tx_anchor);
init_usb_anchor(&priv->int_anchor);
@@ -6080,10 +6085,6 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
goto exit;
}
- ret = rtl8xxxu_init_device(hw);
- if (ret)
- goto exit;
-
hw->wiphy->max_scan_ssids = 1;
hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
--
2.10.1
^ permalink raw reply related
* [PATCH 0/2] rtl8xxxu: Fix allows wpa_supplicant to authenticate
From: John Heenan @ 2016-10-30 10:20 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
With the current kernel release, wpa_supplicant results in authentication failure
with a Cube i9 tablet (a Surface Pro like device):
Successfully initialized wpa_supplicant
wlp0s20f0u7i2: SME: Trying to authenticate with 10:fe:ed:62:7a:78 (SSID='localre' freq=2417 MHz)
wlp0s20f0u7i2: SME: Trying to authenticate with 10:fe:ed:62:7a:78 (SSID='localre' freq=2417 MHz)
wlp0s20f0u7i2: SME: Trying to authenticate with 10:fe:ed:62:7a:78 (SSID='localre' freq=2417 MHz)
wlp0s20f0u7i2: SME: Trying to authenticate with 10:fe:ed:62:7a:78 (SSID='localre' freq=2417 MHz)
wlp0s20f0u7i2: CTRL-EVENT-SSID-TEMP-DISABLED id=0 ssid="localre" auth_failures=1 duration=10 reason=CONN_FAILED
There is a workaround: that ONLY works once per invocation of wpa_supplicant:
rmmod rtl8xxxu
modprobe rtl8xxxu
The follwing two patches result in reliable behaviour, without a workaround,
of wpa_supplicant:
Successfully initialized wpa_supplicant
wlp0s20f0u7i2: SME: Trying to authenticate with 10:fe:ed:62:7a:78 (SSID='localre' freq=2417 MHz)
wlp0s20f0u7i2: Trying to associate with 10:fe:ed:62:7a:78 (SSID='localre' freq=2417 MHz)
wlp0s20f0u7i2: Associated with 10:fe:ed:62:7a:78
wlp0s20f0u7i2: CTRL-EVENT-SUBNET-STATUS-UPDATE status=0
wlp0s20f0u7i2: WPA: Key negotiation completed with 10:fe:ed:62:7a:78 [PTK=CCMP GTK=CCMP]
wlp0s20f0u7i2: CTRL-EVENT-CONNECTED - Connection to 10:fe:ed:62:7a:78 completed [id=0 id_str=]
The patches are for kernel tree:
git://git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git
branch: rtl8xxxu-devel
The first patch moves init code so that each time wpa_supplicant is invoked
there is similar init behaviour as the alternative working rtl8xxxu driver
https://github.com/lwfinger/rtl8723bu
The second patch enables more complete initialisation to occur. There are three issues:
1. The value returned by "rtl8xxxu_read8(priv, REG_CR);", to set macpower, is never 0xef.
The value is either 0x01 (first time with wpa_supplcant after modprobe) or 0x00
(re executing wpa_supplicant)
2. Trying to use the value 0x00 or 0x01 retutned to determine macpower setting always
resulted in failure
3. At the very least 'rtl8xxxu_init_queue_reserved_page(priv);' must always
be invoked, even if not all of the extra init sequence arising from setting
macpower to false is run.
Patched code with a suitable Makefile will be available from
https://github.com/johnheenan/rtl8xxxu for testing by Cube i9 owners
John Heenan (2):
rtl8xxxu: Fix for authentication failure
rtl8xxxu: Fix for bogus data used to determine macpower
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--
2.10.1
^ permalink raw reply
* Re: [PATCH for-next 00/14][PULL request] Mellanox mlx5 core driver updates 2016-10-25
From: Saeed Mahameed @ 2016-10-30 9:59 UTC (permalink / raw)
To: David Miller
Cc: Saeed Mahameed, Doug Ledford, Linux Netdev List,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Or Gerlitz, Leon Romanovsky,
Tal Alon, Matan Barak
In-Reply-To: <20161028.135309.1712496950641242201.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Fri, Oct 28, 2016 at 7:53 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
>
> I really disalike pull requests of this form.
>
> You add lots of datastructures and helper functions but no actual
> users of these facilities to the driver.
>
> Do this instead:
>
> 1) Add TSAR infrastructure
> 2) Add use of TSAR facilities to the driver
>
> That's one pull request.
>
> I don't care if this is hard, or if there are entanglements with
> Infiniband or whatever, you must submit changes in this manner.
>
It is not hard, it is just not right, we have lots of IB and ETH
features that we would like to submit in the same kernel cycle,
with your suggestion I will have to almost submit every feature (core
infrastructure and netdev/RDMA usage)
to you and Doug. Same for rdma features, you will receive PULL
request for them as well,
I am sure you and the netdev list don't need such noise. do not
forget that this will slow down mlx5 progress since
netde will block rdma and vise-versa.
> I will not accept additions to a driver that don't even get really
> used.
For logic/helper functions containing patches such as "Add TSAR
infrastructure" I agree and i can find a way to move some code around
to
avoid future conflicts and remove them from such pull requests.
but you need to at least accept hardware related structures
infrastructure patches for shared code such as
include/linux/mlx5/mlx5_ifc.h where we have only hardware definitions
and those patches are really minimal.
So bottom line, I will do my best to ensure future PULL requests will
contain only include/linux/mlx5/*.h hardware related definitions
or fully implemented features.
Can we agree on that ?
Thanks,
Saeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* pull-request: wireless-drivers-next 2016-10-30
From: Kalle Valo @ 2016-10-30 9:20 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
Hi Dave,
few fixes for 4.9. I tagged this on the plane over a slow mosh
connection while travelling to Plumbers so I might have done something
wrong, please check more carefully than usually. For example I had to
redo the signed tag because of some whitespace damage.
Please let me know if there are any problems.
Kalle
The following changes since commit 67f0160fe34ec5391a428603b9832c9f99d8f3a1:
MAINTAINERS: Update qlogic networking drivers (2016-10-26 23:29:12 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2016-10-30
for you to fetch changes up to d3532ea6ce4ea501e421d130555e59edc2945f99:
brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap (2016-10-27 18:04:54 +0300)
----------------------------------------------------------------
wireless-drivers fixes for 4.9
iwlwifi
* some fixes for suspend/resume with unified FW images
* a fix for a false-positive lockdep report
* a fix for multi-queue that caused an unnecessary 1 second latency
* a fix for an ACPI parsing bug that caused a misleading error message
brcmfmac
* fix a variable uninitialised warning in brcmf_cfg80211_start_ap()
----------------------------------------------------------------
Arnd Bergmann (1):
brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
Haim Dreyfuss (1):
iwlwifi: mvm: comply with fw_restart mod param on suspend
Johannes Berg (1):
iwlwifi: pcie: mark command queue lock with separate lockdep class
Kalle Valo (1):
Merge tag 'iwlwifi-for-kalle-2015-10-25' of git://git.kernel.org/.../iwlwifi/iwlwifi-fixes
Luca Coelho (4):
iwlwifi: mvm: use ssize_t for len in iwl_debugfs_mem_read()
iwlwifi: mvm: fix d3_test with unified D0/D3 images
iwlwifi: pcie: fix SPLC structure parsing
iwlwifi: mvm: fix netdetect starting/stopping for unified images
Sara Sharon (1):
iwlwifi: mvm: wake the wait queue when the RX sync counter is zero
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/d3.c | 49 +++++++++---
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 4 +-
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 3 +-
drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 3 +-
drivers/net/wireless/intel/iwlwifi/mvm/scan.c | 33 ++++++--
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 79 ++++++++++++--------
drivers/net/wireless/intel/iwlwifi/pcie/tx.c | 8 ++
10 files changed, 129 insertions(+), 54 deletions(-)
^ permalink raw reply
* [patch net] mlxsw: spectrum: Fix incorrect reuse of MID entries
From: Jiri Pirko @ 2016-10-30 9:09 UTC (permalink / raw)
To: netdev; +Cc: davem, eladr, idosch, yotamg, nogahf, ogerlitz
From: Ido Schimmel <idosch@mellanox.com>
In the device, a MID entry represents a group of local ports, which can
later be bound to a MDB entry.
The lookup of an existing MID entry is currently done using the provided
MC MAC address and VID, from the Linux bridge. However, this can result
in an incorrect reuse of the same MID index in different VLAN-unaware
bridges (same IP MC group and VID 0).
Fix this by performing the lookup based on FID instead of VID, which is
unique across different bridges.
Fixes: 3a49b4fde2a1 ("mlxsw: Adding layer 2 multicast support")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Elad Raz <eladr@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 14 +++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 9b22863..97bbc1d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -115,7 +115,7 @@ struct mlxsw_sp_rif {
struct mlxsw_sp_mid {
struct list_head list;
unsigned char addr[ETH_ALEN];
- u16 vid;
+ u16 fid;
u16 mid;
unsigned int ref_count;
};
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 5e00c79..1e2c8ec 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -929,12 +929,12 @@ static int mlxsw_sp_port_smid_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 mid,
static struct mlxsw_sp_mid *__mlxsw_sp_mc_get(struct mlxsw_sp *mlxsw_sp,
const unsigned char *addr,
- u16 vid)
+ u16 fid)
{
struct mlxsw_sp_mid *mid;
list_for_each_entry(mid, &mlxsw_sp->br_mids.list, list) {
- if (ether_addr_equal(mid->addr, addr) && mid->vid == vid)
+ if (ether_addr_equal(mid->addr, addr) && mid->fid == fid)
return mid;
}
return NULL;
@@ -942,7 +942,7 @@ static struct mlxsw_sp_mid *__mlxsw_sp_mc_get(struct mlxsw_sp *mlxsw_sp,
static struct mlxsw_sp_mid *__mlxsw_sp_mc_alloc(struct mlxsw_sp *mlxsw_sp,
const unsigned char *addr,
- u16 vid)
+ u16 fid)
{
struct mlxsw_sp_mid *mid;
u16 mid_idx;
@@ -958,7 +958,7 @@ static struct mlxsw_sp_mid *__mlxsw_sp_mc_alloc(struct mlxsw_sp *mlxsw_sp,
set_bit(mid_idx, mlxsw_sp->br_mids.mapped);
ether_addr_copy(mid->addr, addr);
- mid->vid = vid;
+ mid->fid = fid;
mid->mid = mid_idx;
mid->ref_count = 0;
list_add_tail(&mid->list, &mlxsw_sp->br_mids.list);
@@ -991,9 +991,9 @@ static int mlxsw_sp_port_mdb_add(struct mlxsw_sp_port *mlxsw_sp_port,
if (switchdev_trans_ph_prepare(trans))
return 0;
- mid = __mlxsw_sp_mc_get(mlxsw_sp, mdb->addr, mdb->vid);
+ mid = __mlxsw_sp_mc_get(mlxsw_sp, mdb->addr, fid);
if (!mid) {
- mid = __mlxsw_sp_mc_alloc(mlxsw_sp, mdb->addr, mdb->vid);
+ mid = __mlxsw_sp_mc_alloc(mlxsw_sp, mdb->addr, fid);
if (!mid) {
netdev_err(dev, "Unable to allocate MC group\n");
return -ENOMEM;
@@ -1137,7 +1137,7 @@ static int mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
u16 mid_idx;
int err = 0;
- mid = __mlxsw_sp_mc_get(mlxsw_sp, mdb->addr, mdb->vid);
+ mid = __mlxsw_sp_mc_get(mlxsw_sp, mdb->addr, fid);
if (!mid) {
netdev_err(dev, "Unable to remove port from MC DB\n");
return -EINVAL;
--
2.5.5
^ permalink raw reply related
* [PATCH net] qede: Fix statistics' strings for Tx/Rx queues
From: Yuval Mintz @ 2016-10-30 8:25 UTC (permalink / raw)
To: netdev, davem; +Cc: Yuval Mintz
When an interface is configured to use Tx/Rx-only queues,
the length of the statistics would be shortened to accomodate only the
statistics required per-each queue, and the values would be provided
accordingly.
However, the strings provided would still contain both Tx and Rx strings
for each one of the queues [regardless of its configuration], which might
lead to out-of-bound access when filling the buffers as well as incorrect
statistics presented.
Fixes: 9a4d7e86acf3 ("qede: Add support for Tx/Rx-only queues.")
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
Hi Dave,
Please consider applying this to `net'.
Thanks,
Yuval
---
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index d230742..8c2bbb2 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -177,16 +177,23 @@ static void qede_get_strings_stats(struct qede_dev *edev, u8 *buf)
for (i = 0, k = 0; i < QEDE_QUEUE_CNT(edev); i++) {
int tc;
- for (j = 0; j < QEDE_NUM_RQSTATS; j++)
- sprintf(buf + (k + j) * ETH_GSTRING_LEN,
- "%d: %s", i, qede_rqstats_arr[j].string);
- k += QEDE_NUM_RQSTATS;
- for (tc = 0; tc < edev->num_tc; tc++) {
- for (j = 0; j < QEDE_NUM_TQSTATS; j++)
+ if (edev->fp_array[i].type & QEDE_FASTPATH_RX) {
+ for (j = 0; j < QEDE_NUM_RQSTATS; j++)
sprintf(buf + (k + j) * ETH_GSTRING_LEN,
- "%d.%d: %s", i, tc,
- qede_tqstats_arr[j].string);
- k += QEDE_NUM_TQSTATS;
+ "%d: %s", i,
+ qede_rqstats_arr[j].string);
+ k += QEDE_NUM_RQSTATS;
+ }
+
+ if (edev->fp_array[i].type & QEDE_FASTPATH_TX) {
+ for (tc = 0; tc < edev->num_tc; tc++) {
+ for (j = 0; j < QEDE_NUM_TQSTATS; j++)
+ sprintf(buf + (k + j) *
+ ETH_GSTRING_LEN,
+ "%d.%d: %s", i, tc,
+ qede_tqstats_arr[j].string);
+ k += QEDE_NUM_TQSTATS;
+ }
}
}
--
1.9.3
^ permalink raw reply related
* Re: Let's do P4
From: Jiri Pirko @ 2016-10-30 7:44 UTC (permalink / raw)
To: John Fastabend
Cc: Jakub Kicinski, netdev, davem, tgraf, jhs, roopa, simon.horman,
ast, daniel, prem, hannes, jbenc, tom, mattyk, idosch, eladr,
yotamg, nogahf, ogerlitz, linville, andy, f.fainelli, dsa,
vivien.didelot, andrew, ivecera, Maciej Żenczykowski
In-Reply-To: <5814D25D.9070200@gmail.com>
Sat, Oct 29, 2016 at 06:46:21PM CEST, john.fastabend@gmail.com wrote:
>On 16-10-29 07:49 AM, Jakub Kicinski wrote:
>> On Sat, 29 Oct 2016 09:53:28 +0200, Jiri Pirko wrote:
>>> Hi all.
>>>
>>> The network world is divided into 2 general types of hw:
>>> 1) network ASICs - network specific silicon, containing things like TCAM
>>> These ASICs are suitable to be programmed by P4.
>>> 2) network processors - basically a general purpose CPUs
>>> These processors are suitable to be programmed by eBPF.
>>>
>>> I believe that by now, the most people came to a conclusion that it is
>>> very difficult to handle both types by either P4 or eBPF. And since
>>> eBPF is part of the kernel, I would like to introduce P4 into kernel
>>> as well. Here's a plan:
>>>
>>> 1) Define P4 intermediate representation
>>> I cannot imagine loading P4 program (c-like syntax text file) into
>>> kernel as is. That means that as the first step, we need find some
>>> intermediate representation. I can imagine someting in a form of AST,
>>> call it "p4ast". I don't really know how to do this exactly though,
>>> it's just an idea.
>>>
>>> In the end there would be a userspace precompiler for this:
>>> $ makep4ast example.p4 example.ast
>>
>> Maybe stating the obvious, but IMHO defining the IR is the hardest part.
>> eBPF *is* the IR, we can compile C, P4 or even JIT Lua to eBPF. The
>> AST/IR for switch pipelines should allow for similar flexibility.
>> Looser coupling would also protect us from changes in spec of the high
>> level language.
>>
>
>Jumping in the middle here. You managed to get an entire thread going
>before I even woke up :)
>
>The problem with eBPF as an IR is that in the universe of eBPF IR
>programs the subset that can be offloaded onto a standard ASIC based
>hardware (non NPU/FPGA/etc) is so small to be almost meaningless IMO.
>
>I tried this for awhile and the result is users have to write very
>targeted eBPF that they "know" will be pattern matched and pushed into
>an ASIC. It can work but its very fragile. When I did this I ended up
>with an eBPF generator for deviceX and an eBPF generator for deviceY
>each with a very specific pattern matching engine in the driver to
>xlate ebpf-deviceX into its asic. Existing ASICs for example usually
>support only one pipeline, only one parser (or require moving mountains
>to change the parse via ucode), only one set of tables, and only one
>deparser/serailizer at the end to build the new packet. Next-gen pieces
>may have some flexibility on the parser side.
>
>There is an interesting resource allocation problem we have that could
>be solved by p4 or devlink where in we want to pre-allocate slices of
>the TCAM for certain match types. I was planning on writing devlink code
>for this because its primarily done at initialization once.
There are 2 resource allocation problems in our hw. One is general
division ot the resources in feature-chunks. That needs to be done
during the ASIC initialization phase. For that, I also plan to utilize
devlink API.
The second one is runtime allocation of tables, and that would be
handled by p4 just fine.
>
>I will note one nice thing about using eBPF however is that you have an
>easy software emulation path via ebpf engine in kernel.
>
>... And merging threads here with Jiri's email ...
>
>> If you do p4>ebpf in userspace, you have 2 apis:
>> 1) to setup sw (in-kernel) p4 datapath, you push bpf.o to kernel
>> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>>
>> Those are 2 apis. Both wrapped up by TC, but still 2 apis.
>>
>> What I believe is correct is to have one api:
>> 1) to setup sw (in-kernel) p4 datapath, you push program.p4ast to kernel
>> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>>
>
>Couple comments around this, first adding yet another IR in the kernel
>and another JIT engine to map that IR on to eBPF or hardware vendor X
>doesn't get me excited. Its really much easier to write these as backend
>objects in LLVM. Not saying it can't be done just saying it is easier
>in LLVM. Also we already have the LLVM code for P4 to LLVM-IR to eBPF.
>In the end this would be a reasonably complex bit of code in
>the kernel only for hardware offload. I have doubts that folks would
>ever use it for software only cases. I'm happy to admit I'm wrong here
>though.
Well for hw offload, every driver has to parse the IR (whatever will it
be in) and program HW accordingly. Similar parsing and translation would
be needed for SW path, to translate into eBPF. I don't think it would be
more complex than in the drivers. Should be fine.
>
>So yes using llvm backends creates two paths a hardware mgmt and sw
>path but in the hardware + software case typical on the edge the
>orchestration and management planes have started to manage the hardware
>and software as two blocks of logic for performance SLA logic. Even on
>the edge it seems in most cases folks are selling SR-IOV ports and
>can't fall back to software and charge for the port. But this is just
>one use case I suspect others where it does make sense.
>
>> In case of 1), the program.p4ast will be either interpreted by new p4
>> interpreter, of translated to bpf and interpreted by that. But this
>> translation code is part of kernel.
>
>Finally a couple historic bits. The Flow-API proposed in Ottawa was
>mechanically generated from an original P4 draft. At the time I was
>working fairly closely with both the hardware and compiler folks. If
>there is interest we could use that as a base IR for hardware. It has
>a simple mapping to/from the original P4 spec. The newer P4 specs are
>significantly more complex by the way.
Yeah, I was also thinking about something similar to your Flow-API,
but we need something more generic I believe.
>
>We also have an emulated path also auto-generated from compiler tools
>that creates eBPF code from the IR so this would give you the software
>fall-back.
Btw, Flow-API was rejected because it was a clean kernel-bypass. In case
of p4, if we do what Thomas is suggesting, having x.bpf for SW and
x.p4ast for HW, that would be the very same kernel-bypass. Therefore I
strongly believe there should be a single kernel API for p4 SW+HW - for
both p4 program insertion and runtime configuration.
>
>It is something we could spin up an RFC in a few weeks if there is some
>agreement here. I'll be traveling though for a week or two but could
>get something out in November.
>
>Thanks,
>John
>
>
>
>
^ permalink raw reply
* Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep
From: Andrey Konovalov @ 2016-10-30 4:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: Cong Wang, Gerrit Renker, David S. Miller, dccp, netdev, LKML,
Dmitry Vyukov, Eric Dumazet
In-Reply-To: <1477764328.7065.284.camel@edumazet-glaptop3.roam.corp.google.com>
Sorry, the warning is still there.
I'm not sure adding sched_annotate_sleep() does anything, since it's
defined as (in case CONFIG_DEBUG_ATOMIC_SLEEP is not set):
# define sched_annotate_sleep() do { } while (0)
On Sat, Oct 29, 2016 at 8:05 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2016-10-29 at 19:59 +0200, Andrey Konovalov wrote:
>> Hi Eric,
>>
>> Tested with both patches applied, still seeing the warning.
>>
>> Thanks!
>
> Arg, sorry, this was at the wrong place.
>
> Thanks for testing !
>
> diff --git a/net/dccp/output.c b/net/dccp/output.c
> index b66c84db0766..2548edff86ff 100644
> --- a/net/dccp/output.c
> +++ b/net/dccp/output.c
> @@ -224,6 +224,11 @@ static int dccp_wait_for_ccid(struct sock *sk, unsigned long delay)
>
> prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
> sk->sk_write_pending++;
> +
> + /* release_sock()/lock_sock() will process socket backlog
> + * from process context. Be prepared to sleep !
> + */
> + sched_annotate_sleep();
> release_sock(sk);
>
> remaining = schedule_timeout(delay);
>
>
^ permalink raw reply
* Re: [PATCH net-next] firewire: net: really fix maximum possible MTU
From: David Miller @ 2016-10-30 3:01 UTC (permalink / raw)
To: stefanr; +Cc: netdev, linux1394-devel, jarod, linux-kernel
In-Reply-To: <20161029221658.03942ff6@kant>
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sat, 29 Oct 2016 22:16:58 +0200
> The maximum unicast datagram size /without/ link fragmentation is
> 4096 - 4 = 4092 (max IEEE 1394 async payload size at >= S800 bus speed,
> minus unfragmented encapssulation header). Max broadcast datagram size
> without fragmentation is 8 bytes less than that (due to GASP header).
>
> The maximum datagram size /with/ link fragmentation is 0xfff = 4095
> for unicast and broadcast. This is because the RFC 2734 fragment
> encapsulation header field for datagram size is only 12 bits wide.
>
> Fixes: 5d48f00d836a('firewire: net: fix maximum possible MTU')
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Applied, thanks.
^ 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