* Re: [PATCH] net: Reset skb to network header in neigh_hh_output
From: Eric Dumazet @ 2016-10-26 0:12 UTC (permalink / raw)
To: Abdelrhman Ahmed; +Cc: davem, netdev, linux-kernel
In-Reply-To: <157fe46f382.10e50a6d8188917.494328765811573491@abahmed.com>
On Wed, 2016-10-26 at 01:57 +0200, Abdelrhman Ahmed wrote:
> > What is the issue you want to fix exactly ?
> > Please describe the use case.
>
> When netfilter hook uses skb_push to add a specific header between network
> header and hardware header.
> For the first time(s) before caching hardware header, this header will be
> removed / overwritten by hardware header due to resetting to network header.
> After using the cached hardware header, this header will be kept as we do not
> reset. I think this behavior is inconsistent, so we need to reset in both cases.
>
> > Otherwise, your fix is in fact adding a critical bug.
>
> Could you explain more as it's not clear to me?
>
Maybe my wording was not good here.
What I intended to say is that the
__skb_pull(skb, skb_network_offset(skb)) might not be at the right
place.
Look at commit e1f165032c8bade3a6bdf546f8faf61fda4dd01c to find the
reason.
>
>
> ---- On Fri, 07 Oct 2016 23:10:56 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote ----
> > On Fri, 2016-10-07 at 16:14 +0200, Abdelrhman Ahmed wrote:
> > > When hardware header is added without using cached one, neigh_resolve_output
> > > and neigh_connected_output reset skb to network header before adding it.
> > > When cached one is used, neigh_hh_output does not reset the skb to network
> > > header.
> > >
> > > The fix is to reset skb to network header before adding cached hardware header
> > > to keep the behavior consistent in all cases.
> >
> > What is the issue you want to fix exactly ?
> >
> > Please describe the use case.
> >
> > I highly suggest you take a look at commit
> >
> > e1f165032c8bade3a6bdf546f8faf61fda4dd01c
> > ("net: Fix skb_under_panic oops in neigh_resolve_output")
> >
> > Otherwise, your fix is in fact adding a critical bug.
> >
> >
> >
>
^ permalink raw reply
* [PATCH net-next] ibmveth: v1 calculate correct gso_size and set gso_type
From: Jon Maxwell @ 2016-10-26 0:09 UTC (permalink / raw)
To: tlfalcon
Cc: benh, paulus, mpe, davem, tom, jarod, hofrat, netdev,
linuxppc-dev, linux-kernel, mleitner, jmaxwell, Jon Maxwell
We recently encountered a bug where a few customers using ibmveth on the
same LPAR hit an issue where a TCP session hung when large receive was
enabled. Closer analysis revealed that the session was stuck because the
one side was advertising a zero window repeatedly.
We narrowed this down to the fact the ibmveth driver did not set gso_size
which is translated by TCP into the MSS later up the stack. The MSS is
used to calculate the TCP window size and as that was abnormally large,
it was calculating a zero window, even although the sockets receive buffer
was completely empty.
We were able to reproduce this and worked with IBM to fix this. Thanks Tom
and Marcelo for all your help and review on this.
The patch fixes both our internal reproduction tests and our customers tests.
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
drivers/net/ethernet/ibm/ibmveth.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 29c05d0..c51717e 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1182,6 +1182,8 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
int frames_processed = 0;
unsigned long lpar_rc;
struct iphdr *iph;
+ bool large_packet = 0;
+ u16 hdr_len = ETH_HLEN + sizeof(struct tcphdr);
restart_poll:
while (frames_processed < budget) {
@@ -1236,10 +1238,28 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
iph->check = 0;
iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
adapter->rx_large_packets++;
+ large_packet = 1;
}
}
}
+ if (skb->len > netdev->mtu) {
+ iph = (struct iphdr *)skb->data;
+ if (be16_to_cpu(skb->protocol) == ETH_P_IP &&
+ iph->protocol == IPPROTO_TCP) {
+ hdr_len += sizeof(struct iphdr);
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+ skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
+ } else if (be16_to_cpu(skb->protocol) == ETH_P_IPV6 &&
+ iph->protocol == IPPROTO_TCP) {
+ hdr_len += sizeof(struct ipv6hdr);
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+ skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
+ }
+ if (!large_packet)
+ adapter->rx_large_packets++;
+ }
+
napi_gro_receive(napi, skb); /* send it up */
netdev->stats.rx_packets++;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: Reset skb to network header in neigh_hh_output
From: Abdelrhman Ahmed @ 2016-10-25 23:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1475874656.28155.268.camel@edumazet-glaptop3.roam.corp.google.com>
> What is the issue you want to fix exactly ?
> Please describe the use case.
When netfilter hook uses skb_push to add a specific header between network
header and hardware header.
For the first time(s) before caching hardware header, this header will be
removed / overwritten by hardware header due to resetting to network header.
After using the cached hardware header, this header will be kept as we do not
reset. I think this behavior is inconsistent, so we need to reset in both cases.
> Otherwise, your fix is in fact adding a critical bug.
Could you explain more as it's not clear to me?
---- On Fri, 07 Oct 2016 23:10:56 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote ----
> On Fri, 2016-10-07 at 16:14 +0200, Abdelrhman Ahmed wrote:
> > When hardware header is added without using cached one, neigh_resolve_output
> > and neigh_connected_output reset skb to network header before adding it.
> > When cached one is used, neigh_hh_output does not reset the skb to network
> > header.
> >
> > The fix is to reset skb to network header before adding cached hardware header
> > to keep the behavior consistent in all cases.
>
> What is the issue you want to fix exactly ?
>
> Please describe the use case.
>
> I highly suggest you take a look at commit
>
> e1f165032c8bade3a6bdf546f8faf61fda4dd01c
> ("net: Fix skb_under_panic oops in neigh_resolve_output")
>
> Otherwise, your fix is in fact adding a critical bug.
>
>
>
^ permalink raw reply
* Re: [PATCH net-next 2/3] bpf: Add new cgroups prog type to enable sock modifications
From: Eric Dumazet @ 2016-10-25 23:39 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, daniel, ast, daniel
In-Reply-To: <1477434613-3169-3-git-send-email-dsa@cumulusnetworks.com>
On Tue, 2016-10-25 at 15:30 -0700, David Ahern wrote:
> Add new cgroup based program type, BPF_PROG_TYPE_CGROUP_SOCK. Similar to
> BPF_PROG_TYPE_CGROUP_SKB programs can be attached to a cgroup and run
> any time a process in the cgroup opens an AF_INET or AF_INET6 socket.
> Currently only sk_bound_dev_if is exported to userspace for modification
> by a bpf program.
>
> This allows a cgroup to be configured such that AF_INET{6} sockets opened
> by processes are automatically bound to a specific device. In turn, this
> enables the running of programs that do not support SO_BINDTODEVICE in a
> specific VRF context / L3 domain.
Does this mean that these programs no longer can use loopback ?
^ permalink raw reply
* Re: [PATCH net-next 2/3] bpf: Add new cgroups prog type to enable sock modifications
From: Daniel Borkmann @ 2016-10-25 23:28 UTC (permalink / raw)
To: David Ahern, netdev; +Cc: daniel, ast
In-Reply-To: <1477434613-3169-3-git-send-email-dsa@cumulusnetworks.com>
On 10/26/2016 12:30 AM, David Ahern wrote:
> Add new cgroup based program type, BPF_PROG_TYPE_CGROUP_SOCK. Similar to
> BPF_PROG_TYPE_CGROUP_SKB programs can be attached to a cgroup and run
> any time a process in the cgroup opens an AF_INET or AF_INET6 socket.
> Currently only sk_bound_dev_if is exported to userspace for modification
> by a bpf program.
>
> This allows a cgroup to be configured such that AF_INET{6} sockets opened
> by processes are automatically bound to a specific device. In turn, this
> enables the running of programs that do not support SO_BINDTODEVICE in a
> specific VRF context / L3 domain.
>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
[...]
> @@ -524,6 +535,10 @@ struct bpf_tunnel_key {
> __u32 tunnel_label;
> };
>
> +struct bpf_sock {
> + __u32 bound_dev_if;
> +};
> +
> /* 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/net/core/filter.c b/net/core/filter.c
> index 4552b8c93b99..775802881b01 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2482,6 +2482,27 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
> .arg5_type = ARG_CONST_STACK_SIZE,
> };
>
> +BPF_CALL_3(bpf_sock_store_u32, struct sock *, sk, u32, offset, u32, val)
> +{
> + u8 *ptr = (u8 *)sk;
> +
> + if (unlikely(offset > sizeof(*sk)))
> + return -EFAULT;
> +
> + *((u32 *)ptr) = val;
> +
> + return 0;
> +}
Seems strange to me. So, this helper allows to overwrite arbitrary memory
of a struct sock instance. Potentially we could crash the kernel.
And in your sock_filter_convert_ctx_access(), you already implement inline
read/write for the context ...
Your demo code does in pseudocode:
r1 = sk
r2 = offsetof(struct bpf_sock, bound_dev_if)
r3 = idx
r1->sk_bound_dev_if = idx
sock_store_u32(r1, r2, r3) // updates sk_bound_dev_if again to idx
return 1
Dropping that helper from the patch, the only thing a program can do here
is to read/write the sk_bound_dev_if helper per cgroup. Hmm ... dunno. So
this really has to be for cgroups v2, right?
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: Refactor cgroups code in prep for new type
From: David Ahern @ 2016-10-25 23:04 UTC (permalink / raw)
To: Daniel Borkmann, netdev; +Cc: daniel, ast
In-Reply-To: <580FE44F.2030403@iogearbox.net>
On 10/25/16 5:01 PM, Daniel Borkmann wrote:
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> index a0ab43f264b0..918c01a6f129 100644
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
>> @@ -117,6 +117,19 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
>> }
>> }
>>
>> +static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
>> + struct bpf_prog *prog)
>> +{
>> + unsigned int offset = skb->data - skb_network_header(skb);
>> + int ret;
>> +
>> + __skb_push(skb, offset);
>> + ret = bpf_prog_run_clear_cb(prog, skb) == 1 ? 0 : -EPERM;
>
> Original code save skb->cb[], this one clears it.
>
ah, it changed in Daniel's v6 to v7 code and I missed it. Will fix. Thanks for pointing it out.
^ permalink raw reply
* [PATCH] uapi: Fix userspace compilation of ip_tables.h/ip6_tables.h in C++ mode
From: Jason Gunthorpe @ 2016-10-25 23:03 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-kernel
The implicit cast from void * is not allowed for C++ compilers, and the
arithmetic on void * generates warnings if a C++ application tries to include
these UAPI headers.
$ g++ -c t.cc
ip_tables.h:221:24: warning: pointer of type 'void *' used in arithmetic
ip_tables.h:221:24: error: invalid conversion from 'void*' to 'xt_entry_target*'
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
include/uapi/linux/netfilter_ipv4/ip_tables.h | 2 +-
include/uapi/linux/netfilter_ipv6/ip6_tables.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/netfilter_ipv4/ip_tables.h b/include/uapi/linux/netfilter_ipv4/ip_tables.h
index d0da53d96d93..4682b18f3f44 100644
--- a/include/uapi/linux/netfilter_ipv4/ip_tables.h
+++ b/include/uapi/linux/netfilter_ipv4/ip_tables.h
@@ -221,7 +221,7 @@ struct ipt_get_entries {
static __inline__ struct xt_entry_target *
ipt_get_target(struct ipt_entry *e)
{
- return (void *)e + e->target_offset;
+ return (struct xt_entry_target *)((__u8 *)e + e->target_offset);
}
/*
diff --git a/include/uapi/linux/netfilter_ipv6/ip6_tables.h b/include/uapi/linux/netfilter_ipv6/ip6_tables.h
index d1b22653daf2..05e0631a6d12 100644
--- a/include/uapi/linux/netfilter_ipv6/ip6_tables.h
+++ b/include/uapi/linux/netfilter_ipv6/ip6_tables.h
@@ -261,7 +261,7 @@ struct ip6t_get_entries {
static __inline__ struct xt_entry_target *
ip6t_get_target(struct ip6t_entry *e)
{
- return (void *)e + e->target_offset;
+ return (struct xt_entry_target *)((__u8 *)e + e->target_offset);
}
/*
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next 1/3] bpf: Refactor cgroups code in prep for new type
From: Daniel Borkmann @ 2016-10-25 23:01 UTC (permalink / raw)
To: David Ahern, netdev; +Cc: daniel, ast
In-Reply-To: <1477434613-3169-2-git-send-email-dsa@cumulusnetworks.com>
On 10/26/2016 12:30 AM, David Ahern wrote:
> Code move only; no functional change intended.
Not quite, see below.
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> kernel/bpf/cgroup.c | 27 ++++++++++++++++++++++-----
> kernel/bpf/syscall.c | 28 +++++++++++++++-------------
> 2 files changed, 37 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index a0ab43f264b0..918c01a6f129 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -117,6 +117,19 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
> }
> }
>
> +static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
> + struct bpf_prog *prog)
> +{
> + unsigned int offset = skb->data - skb_network_header(skb);
> + int ret;
> +
> + __skb_push(skb, offset);
> + ret = bpf_prog_run_clear_cb(prog, skb) == 1 ? 0 : -EPERM;
Original code save skb->cb[], this one clears it.
> + __skb_pull(skb, offset);
> +
> + return ret;
> +}
> +
> /**
> * __cgroup_bpf_run_filter() - Run a program for packet filtering
> * @sk: The socken sending or receiving traffic
> @@ -153,11 +166,15 @@ int __cgroup_bpf_run_filter(struct sock *sk,
>
> prog = rcu_dereference(cgrp->bpf.effective[type]);
> if (prog) {
> - unsigned int offset = skb->data - skb_network_header(skb);
> -
> - __skb_push(skb, offset);
> - ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM;
> - __skb_pull(skb, offset);
> + switch (type) {
> + case BPF_CGROUP_INET_INGRESS:
> + case BPF_CGROUP_INET_EGRESS:
> + ret = __cgroup_bpf_run_filter_skb(skb, prog);
> + break;
> + /* make gcc happy else complains about missing enum value */
> + default:
> + return 0;
> + }
> }
^ permalink raw reply
* [PATCH net] bpf: fix samples to add fake KBUILD_MODNAME
From: Daniel Borkmann @ 2016-10-25 22:37 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, netdev, Daniel Borkmann
Some of the sample files are causing issues when they are loaded with tc
and cls_bpf, meaning tc bails out while trying to parse the resulting ELF
file as program/map/etc sections are not present, which can be easily
spotted with readelf(1).
Currently, BPF samples are including some of the kernel headers and mid
term we should change them to refrain from this, really. When dynamic
debugging is enabled, we bail out due to undeclared KBUILD_MODNAME, which
is easily overlooked in the build as clang spills this along with other
noisy warnings from various header includes, and llc still generates an
ELF file with mentioned characteristics. For just playing around with BPF
examples, this can be a bit of a hurdle to take.
Just add a fake KBUILD_MODNAME as a band-aid to fix the issue, same is
done in xdp*_kern samples already.
Fixes: 65d472fb007d ("samples/bpf: add 'pointer to packet' tests")
Fixes: 6afb1e28b859 ("samples/bpf: Add tunnel set/get tests.")
Fixes: a3f74617340b ("cgroup: bpf: Add an example to do cgroup checking in BPF")
Reported-by: Chandrasekar Kannan <ckannan@console.to>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
samples/bpf/parse_ldabs.c | 1 +
samples/bpf/parse_simple.c | 1 +
samples/bpf/parse_varlen.c | 1 +
samples/bpf/tcbpf1_kern.c | 1 +
samples/bpf/tcbpf2_kern.c | 1 +
samples/bpf/test_cgrp2_tc_kern.c | 1 +
6 files changed, 6 insertions(+)
diff --git a/samples/bpf/parse_ldabs.c b/samples/bpf/parse_ldabs.c
index d175501..6db6b21 100644
--- a/samples/bpf/parse_ldabs.c
+++ b/samples/bpf/parse_ldabs.c
@@ -4,6 +4,7 @@
* modify it under the terms of version 2 of the GNU General Public
* License as published by the Free Software Foundation.
*/
+#define KBUILD_MODNAME "foo"
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <linux/in.h>
diff --git a/samples/bpf/parse_simple.c b/samples/bpf/parse_simple.c
index cf2511c..10af53d 100644
--- a/samples/bpf/parse_simple.c
+++ b/samples/bpf/parse_simple.c
@@ -4,6 +4,7 @@
* modify it under the terms of version 2 of the GNU General Public
* License as published by the Free Software Foundation.
*/
+#define KBUILD_MODNAME "foo"
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <linux/in.h>
diff --git a/samples/bpf/parse_varlen.c b/samples/bpf/parse_varlen.c
index edab34d..95c1632 100644
--- a/samples/bpf/parse_varlen.c
+++ b/samples/bpf/parse_varlen.c
@@ -4,6 +4,7 @@
* modify it under the terms of version 2 of the GNU General Public
* License as published by the Free Software Foundation.
*/
+#define KBUILD_MODNAME "foo"
#include <linux/if_ether.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
diff --git a/samples/bpf/tcbpf1_kern.c b/samples/bpf/tcbpf1_kern.c
index fa051b3..274c884 100644
--- a/samples/bpf/tcbpf1_kern.c
+++ b/samples/bpf/tcbpf1_kern.c
@@ -1,3 +1,4 @@
+#define KBUILD_MODNAME "foo"
#include <uapi/linux/bpf.h>
#include <uapi/linux/if_ether.h>
#include <uapi/linux/if_packet.h>
diff --git a/samples/bpf/tcbpf2_kern.c b/samples/bpf/tcbpf2_kern.c
index 3303bb8..9c823a6 100644
--- a/samples/bpf/tcbpf2_kern.c
+++ b/samples/bpf/tcbpf2_kern.c
@@ -5,6 +5,7 @@
* modify it under the terms of version 2 of the GNU General Public
* License as published by the Free Software Foundation.
*/
+#define KBUILD_MODNAME "foo"
#include <uapi/linux/bpf.h>
#include <uapi/linux/if_ether.h>
#include <uapi/linux/if_packet.h>
diff --git a/samples/bpf/test_cgrp2_tc_kern.c b/samples/bpf/test_cgrp2_tc_kern.c
index 10ff734..1547b36 100644
--- a/samples/bpf/test_cgrp2_tc_kern.c
+++ b/samples/bpf/test_cgrp2_tc_kern.c
@@ -4,6 +4,7 @@
* modify it under the terms of version 2 of the GNU General Public
* License as published by the Free Software Foundation.
*/
+#define KBUILD_MODNAME "foo"
#include <uapi/linux/if_ether.h>
#include <uapi/linux/in6.h>
#include <uapi/linux/ipv6.h>
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net] inet: Fix missing return value in inet6_hash
From: Soheil Hassas Yeganeh @ 2016-10-25 22:36 UTC (permalink / raw)
To: Craig Gallek; +Cc: David Miller, netdev
In-Reply-To: <1477433329-165391-1-git-send-email-kraigatgoog@gmail.com>
On Tue, Oct 25, 2016 at 6:08 PM, Craig Gallek <kraigatgoog@gmail.com> wrote:
> From: Craig Gallek <kraig@google.com>
>
> As part of a series to implement faster SO_REUSEPORT lookups,
> commit 086c653f5862 ("sock: struct proto hash function may error")
> added return values to protocol hash functions and
> commit 496611d7b5ea ("inet: create IPv6-equivalent inet_hash function")
> implemented a new hash function for IPv6. However, the latter does
> not respect the former's convention.
>
> This properly propagates the hash errors in the IPv6 case.
>
> Fixes: 496611d7b5ea ("inet: create IPv6-equivalent inet_hash function")
> Reported-by: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Craig Gallek <kraig@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> net/ipv6/inet6_hashtables.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
> index 2fd0374a35b1..02761c9fe43e 100644
> --- a/net/ipv6/inet6_hashtables.c
> +++ b/net/ipv6/inet6_hashtables.c
> @@ -264,13 +264,15 @@ EXPORT_SYMBOL_GPL(inet6_hash_connect);
>
> int inet6_hash(struct sock *sk)
> {
> + int err = 0;
> +
> if (sk->sk_state != TCP_CLOSE) {
> local_bh_disable();
> - __inet_hash(sk, NULL, ipv6_rcv_saddr_equal);
> + err = __inet_hash(sk, NULL, ipv6_rcv_saddr_equal);
> local_bh_enable();
> }
>
> - return 0;
> + return err;
> }
> EXPORT_SYMBOL_GPL(inet6_hash);
Thanks for the fix!
> --
> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* [PATCH net-next 3/3] samples: bpf: add userspace example for modifying sk_bound_dev_if
From: David Ahern @ 2016-10-25 22:30 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, David Ahern
In-Reply-To: <1477434613-3169-1-git-send-email-dsa@cumulusnetworks.com>
Add a simple program to demonstrate the ability to attach a bpf program
to a cgroup that sets sk_bound_dev_if for AF_INET{6} sockets when they
are created.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
samples/bpf/Makefile | 2 ++
samples/bpf/bpf_helpers.h | 2 ++
samples/bpf/test_cgrp2_sock.c | 84 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 88 insertions(+)
create mode 100644 samples/bpf/test_cgrp2_sock.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 2624d5d7ce8b..ec4ef37a2dbc 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -22,6 +22,7 @@ hostprogs-y += map_perf_test
hostprogs-y += test_overhead
hostprogs-y += test_cgrp2_array_pin
hostprogs-y += test_cgrp2_attach
+hostprogs-y += test_cgrp2_sock
hostprogs-y += xdp1
hostprogs-y += xdp2
hostprogs-y += test_current_task_under_cgroup
@@ -48,6 +49,7 @@ map_perf_test-objs := bpf_load.o libbpf.o map_perf_test_user.o
test_overhead-objs := bpf_load.o libbpf.o test_overhead_user.o
test_cgrp2_array_pin-objs := libbpf.o test_cgrp2_array_pin.o
test_cgrp2_attach-objs := libbpf.o test_cgrp2_attach.o
+test_cgrp2_sock-objs := libbpf.o test_cgrp2_sock.o
xdp1-objs := bpf_load.o libbpf.o xdp1_user.o
# reuse xdp1 source intentionally
xdp2-objs := bpf_load.o libbpf.o xdp1_user.o
diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
index 90f44bd2045e..7d95c9af3681 100644
--- a/samples/bpf/bpf_helpers.h
+++ b/samples/bpf/bpf_helpers.h
@@ -88,6 +88,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_sock_store_u32)(void *ctx, __u32 off, __u32 val) =
+ (void *) BPF_FUNC_sock_store_u32;
#if defined(__x86_64__)
diff --git a/samples/bpf/test_cgrp2_sock.c b/samples/bpf/test_cgrp2_sock.c
new file mode 100644
index 000000000000..1fab10a08846
--- /dev/null
+++ b/samples/bpf/test_cgrp2_sock.c
@@ -0,0 +1,84 @@
+/* eBPF example program:
+ *
+ * - Loads eBPF program
+ *
+ * The eBPF program sets the sk_bound_dev_if index in new AF_INET{6}
+ * sockets opened by processes in the cgroup.
+ *
+ * - Attaches the new program to a cgroup using BPF_PROG_ATTACH
+ */
+
+#define _GNU_SOURCE
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <string.h>
+#include <unistd.h>
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/bpf.h>
+
+#include "libbpf.h"
+
+static int prog_load(int idx)
+{
+ struct bpf_insn prog[] = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_MOV64_IMM(BPF_REG_3, idx),
+ BPF_MOV64_IMM(BPF_REG_2, offsetof(struct bpf_sock, bound_dev_if)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_3, offsetof(struct bpf_sock, bound_dev_if)),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_sock_store_u32),
+ BPF_MOV64_IMM(BPF_REG_0, 1), /* r0 = verdict */
+ BPF_EXIT_INSN(),
+ };
+
+ return bpf_prog_load(BPF_PROG_TYPE_CGROUP_SOCK,
+ prog, sizeof(prog), "GPL", 0);
+}
+
+static int usage(const char *argv0)
+{
+ printf("Usage: %s <cg-path> device-index\n", argv0);
+ return EXIT_FAILURE;
+}
+
+int main(int argc, char **argv)
+{
+ int cg_fd, prog_fd, ret;
+ int idx = 0;
+
+ if (argc < 2)
+ return usage(argv[0]);
+
+ idx = atoi(argv[2]);
+ if (!idx) {
+ printf("Invalid device index\n");
+ return EXIT_FAILURE;
+ }
+
+ cg_fd = open(argv[1], O_DIRECTORY | O_RDONLY);
+ if (cg_fd < 0) {
+ printf("Failed to open cgroup path: '%s'\n", strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ prog_fd = prog_load(idx);
+ printf("Output from kernel verifier:\n%s\n-------\n", bpf_log_buf);
+
+ if (prog_fd < 0) {
+ printf("Failed to load prog: '%s'\n", strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ ret = bpf_prog_detach(cg_fd, BPF_CGROUP_INET_SOCK_CREATE);
+ ret = bpf_prog_attach(prog_fd, cg_fd, BPF_CGROUP_INET_SOCK_CREATE);
+ if (ret < 0) {
+ printf("Failed to attach prog to cgroup: '%s'\n",
+ strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 2/3] bpf: Add new cgroups prog type to enable sock modifications
From: David Ahern @ 2016-10-25 22:30 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, David Ahern
In-Reply-To: <1477434613-3169-1-git-send-email-dsa@cumulusnetworks.com>
Add new cgroup based program type, BPF_PROG_TYPE_CGROUP_SOCK. Similar to
BPF_PROG_TYPE_CGROUP_SKB programs can be attached to a cgroup and run
any time a process in the cgroup opens an AF_INET or AF_INET6 socket.
Currently only sk_bound_dev_if is exported to userspace for modification
by a bpf program.
This allows a cgroup to be configured such that AF_INET{6} sockets opened
by processes are automatically bound to a specific device. In turn, this
enables the running of programs that do not support SO_BINDTODEVICE in a
specific VRF context / L3 domain.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
include/linux/filter.h | 2 +-
include/uapi/linux/bpf.h | 15 ++++++++
kernel/bpf/cgroup.c | 9 +++++
kernel/bpf/syscall.c | 4 +++
net/core/filter.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
net/core/sock.c | 7 ++++
6 files changed, 128 insertions(+), 1 deletion(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 1f09c521adfe..808e158742a2 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -408,7 +408,7 @@ struct bpf_prog {
enum bpf_prog_type type; /* Type of BPF program */
struct bpf_prog_aux *aux; /* Auxiliary fields */
struct sock_fprog_kern *orig_prog; /* Original BPF program */
- unsigned int (*bpf_func)(const struct sk_buff *skb,
+ unsigned int (*bpf_func)(const void *ctx,
const struct bpf_insn *filter);
/* Instructions for interpreter */
union {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6b62ee9a2f78..ce5283f221e7 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -99,11 +99,13 @@ enum bpf_prog_type {
BPF_PROG_TYPE_XDP,
BPF_PROG_TYPE_PERF_EVENT,
BPF_PROG_TYPE_CGROUP_SKB,
+ BPF_PROG_TYPE_CGROUP_SOCK,
};
enum bpf_attach_type {
BPF_CGROUP_INET_INGRESS,
BPF_CGROUP_INET_EGRESS,
+ BPF_CGROUP_INET_SOCK_CREATE,
__MAX_BPF_ATTACH_TYPE
};
@@ -449,6 +451,15 @@ enum bpf_func_id {
*/
BPF_FUNC_get_numa_node_id,
+ /**
+ * sock_store_u32(sk, offset, val) - store bytes into sock
+ * @sk: pointer to sock
+ * @offset: offset within sock
+ * @val: value to write
+ * Return: 0 on success
+ */
+ BPF_FUNC_sock_store_u32,
+
__BPF_FUNC_MAX_ID,
};
@@ -524,6 +535,10 @@ struct bpf_tunnel_key {
__u32 tunnel_label;
};
+struct bpf_sock {
+ __u32 bound_dev_if;
+};
+
/* 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/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 918c01a6f129..4fcb58013a3a 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -117,6 +117,12 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
}
}
+static int __cgroup_bpf_run_filter_sk_create(struct sock *sk,
+ struct bpf_prog *prog)
+{
+ return prog->bpf_func(sk, prog->insnsi) == 1 ? 0 : -EPERM;
+}
+
static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
struct bpf_prog *prog)
{
@@ -171,6 +177,9 @@ int __cgroup_bpf_run_filter(struct sock *sk,
case BPF_CGROUP_INET_EGRESS:
ret = __cgroup_bpf_run_filter_skb(skb, prog);
break;
+ case BPF_CGROUP_INET_SOCK_CREATE:
+ ret = __cgroup_bpf_run_filter_sk_create(sk, prog);
+ break;
/* make gcc happy else complains about missing enum value */
default:
return 0;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 9abc88deabbc..3b7e30e28cd3 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -844,6 +844,9 @@ static int bpf_prog_attach(const union bpf_attr *attr)
ptype = BPF_PROG_TYPE_CGROUP_SKB;
break;
+ case BPF_CGROUP_INET_SOCK_CREATE:
+ ptype = BPF_PROG_TYPE_CGROUP_SOCK;
+ break;
default:
return -EINVAL;
}
@@ -879,6 +882,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
switch (attr->attach_type) {
case BPF_CGROUP_INET_INGRESS:
case BPF_CGROUP_INET_EGRESS:
+ case BPF_CGROUP_INET_SOCK_CREATE:
cgrp = cgroup_get_from_fd(attr->target_fd);
if (IS_ERR(cgrp))
return PTR_ERR(cgrp);
diff --git a/net/core/filter.c b/net/core/filter.c
index 4552b8c93b99..775802881b01 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2482,6 +2482,27 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
.arg5_type = ARG_CONST_STACK_SIZE,
};
+BPF_CALL_3(bpf_sock_store_u32, struct sock *, sk, u32, offset, u32, val)
+{
+ u8 *ptr = (u8 *)sk;
+
+ if (unlikely(offset > sizeof(*sk)))
+ return -EFAULT;
+
+ *((u32 *)ptr) = val;
+
+ return 0;
+}
+
+static const struct bpf_func_proto bpf_sock_store_u32_proto = {
+ .func = bpf_sock_store_u32,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+};
+
static const struct bpf_func_proto *
sk_filter_func_proto(enum bpf_func_id func_id)
{
@@ -2593,6 +2614,17 @@ cg_skb_func_proto(enum bpf_func_id func_id)
}
}
+static const struct bpf_func_proto *
+cg_sock_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ case BPF_FUNC_sock_store_u32:
+ return &bpf_sock_store_u32_proto;
+ default:
+ return NULL;
+ }
+}
+
static bool __is_valid_access(int off, int size, enum bpf_access_type type)
{
if (off < 0 || off >= sizeof(struct __sk_buff))
@@ -2630,6 +2662,30 @@ static bool sk_filter_is_valid_access(int off, int size,
return __is_valid_access(off, size, type);
}
+static bool sock_filter_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ enum bpf_reg_type *reg_type)
+{
+ if (type == BPF_WRITE) {
+ switch (off) {
+ case offsetof(struct bpf_sock, bound_dev_if):
+ break;
+ default:
+ return false;
+ }
+ }
+
+ if (off < 0 || off >= sizeof(struct bpf_sock))
+ return false;
+ /* The verifier guarantees that size > 0. */
+ if (off % size != 0)
+ return false;
+ if (size != sizeof(__u32))
+ return false;
+
+ return true;
+}
+
static int tc_cls_act_prologue(struct bpf_insn *insn_buf, bool direct_write,
const struct bpf_prog *prog)
{
@@ -2888,6 +2944,30 @@ static u32 sk_filter_convert_ctx_access(enum bpf_access_type type, int dst_reg,
return insn - insn_buf;
}
+static u32 sock_filter_convert_ctx_access(enum bpf_access_type type,
+ int dst_reg, int src_reg,
+ int ctx_off,
+ struct bpf_insn *insn_buf,
+ struct bpf_prog *prog)
+{
+ struct bpf_insn *insn = insn_buf;
+
+ switch (ctx_off) {
+ case offsetof(struct bpf_sock, bound_dev_if):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock, sk_bound_dev_if) != 4);
+
+ if (type == BPF_WRITE)
+ *insn++ = BPF_STX_MEM(BPF_W, dst_reg, src_reg,
+ offsetof(struct sock, sk_bound_dev_if));
+ else
+ *insn++ = BPF_LDX_MEM(BPF_W, dst_reg, src_reg,
+ offsetof(struct sock, sk_bound_dev_if));
+ break;
+ }
+
+ return insn - insn_buf;
+}
+
static u32 tc_cls_act_convert_ctx_access(enum bpf_access_type type, int dst_reg,
int src_reg, int ctx_off,
struct bpf_insn *insn_buf,
@@ -2961,6 +3041,12 @@ static const struct bpf_verifier_ops cg_skb_ops = {
.convert_ctx_access = sk_filter_convert_ctx_access,
};
+static const struct bpf_verifier_ops cg_sock_ops = {
+ .get_func_proto = cg_sock_func_proto,
+ .is_valid_access = sock_filter_is_valid_access,
+ .convert_ctx_access = sock_filter_convert_ctx_access,
+};
+
static struct bpf_prog_type_list sk_filter_type __read_mostly = {
.ops = &sk_filter_ops,
.type = BPF_PROG_TYPE_SOCKET_FILTER,
@@ -2986,6 +3072,11 @@ static struct bpf_prog_type_list cg_skb_type __read_mostly = {
.type = BPF_PROG_TYPE_CGROUP_SKB,
};
+static struct bpf_prog_type_list cg_sock_type __read_mostly = {
+ .ops = &cg_sock_ops,
+ .type = BPF_PROG_TYPE_CGROUP_SOCK
+};
+
static int __init register_sk_filter_ops(void)
{
bpf_register_prog_type(&sk_filter_type);
@@ -2993,6 +3084,7 @@ static int __init register_sk_filter_ops(void)
bpf_register_prog_type(&sched_act_type);
bpf_register_prog_type(&xdp_type);
bpf_register_prog_type(&cg_skb_type);
+ bpf_register_prog_type(&cg_sock_type);
return 0;
}
diff --git a/net/core/sock.c b/net/core/sock.c
index d8e4532e89e7..936f221cc6c6 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1404,6 +1404,13 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
cgroup_sk_alloc(&sk->sk_cgrp_data);
sock_update_classid(&sk->sk_cgrp_data);
sock_update_netprioidx(&sk->sk_cgrp_data);
+
+ if (!kern &&
+ cgroup_bpf_run_filter(sk, NULL,
+ BPF_CGROUP_INET_SOCK_CREATE)) {
+ sk_free(sk);
+ sk = NULL;
+ }
}
return sk;
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 1/3] bpf: Refactor cgroups code in prep for new type
From: David Ahern @ 2016-10-25 22:30 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, David Ahern
In-Reply-To: <1477434613-3169-1-git-send-email-dsa@cumulusnetworks.com>
Code move only; no functional change intended.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
kernel/bpf/cgroup.c | 27 ++++++++++++++++++++++-----
kernel/bpf/syscall.c | 28 +++++++++++++++-------------
2 files changed, 37 insertions(+), 18 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index a0ab43f264b0..918c01a6f129 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -117,6 +117,19 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
}
}
+static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
+ struct bpf_prog *prog)
+{
+ unsigned int offset = skb->data - skb_network_header(skb);
+ int ret;
+
+ __skb_push(skb, offset);
+ ret = bpf_prog_run_clear_cb(prog, skb) == 1 ? 0 : -EPERM;
+ __skb_pull(skb, offset);
+
+ return ret;
+}
+
/**
* __cgroup_bpf_run_filter() - Run a program for packet filtering
* @sk: The socken sending or receiving traffic
@@ -153,11 +166,15 @@ int __cgroup_bpf_run_filter(struct sock *sk,
prog = rcu_dereference(cgrp->bpf.effective[type]);
if (prog) {
- unsigned int offset = skb->data - skb_network_header(skb);
-
- __skb_push(skb, offset);
- ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM;
- __skb_pull(skb, offset);
+ switch (type) {
+ case BPF_CGROUP_INET_INGRESS:
+ case BPF_CGROUP_INET_EGRESS:
+ ret = __cgroup_bpf_run_filter_skb(skb, prog);
+ break;
+ /* make gcc happy else complains about missing enum value */
+ default:
+ return 0;
+ }
}
rcu_read_unlock();
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 1814c010ace6..9abc88deabbc 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -828,6 +828,7 @@ static int bpf_obj_get(const union bpf_attr *attr)
static int bpf_prog_attach(const union bpf_attr *attr)
{
+ enum bpf_prog_type ptype = BPF_PROG_TYPE_UNSPEC;
struct bpf_prog *prog;
struct cgroup *cgrp;
@@ -840,25 +841,26 @@ static int bpf_prog_attach(const union bpf_attr *attr)
switch (attr->attach_type) {
case BPF_CGROUP_INET_INGRESS:
case BPF_CGROUP_INET_EGRESS:
- prog = bpf_prog_get_type(attr->attach_bpf_fd,
- BPF_PROG_TYPE_CGROUP_SKB);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
-
- cgrp = cgroup_get_from_fd(attr->target_fd);
- if (IS_ERR(cgrp)) {
- bpf_prog_put(prog);
- return PTR_ERR(cgrp);
- }
-
- cgroup_bpf_update(cgrp, prog, attr->attach_type);
- cgroup_put(cgrp);
+ ptype = BPF_PROG_TYPE_CGROUP_SKB;
break;
default:
return -EINVAL;
}
+ prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ cgrp = cgroup_get_from_fd(attr->target_fd);
+ if (IS_ERR(cgrp)) {
+ bpf_prog_put(prog);
+ return PTR_ERR(cgrp);
+ }
+
+ cgroup_bpf_update(cgrp, prog, attr->attach_type);
+ cgroup_put(cgrp);
+
return 0;
}
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 0/3] Add bpf support to set sk_bound_dev_if
From: David Ahern @ 2016-10-25 22:30 UTC (permalink / raw)
To: netdev; +Cc: daniel, ast, daniel, David Ahern
The recently added VRF support in Linux leverages the bind-to-device
API for programs to specify an L3 domain for a socket. While
SO_BINDTODEVICE has been around for ages, not every ipv4/ipv6 capable
program has support for it. Even for those programs that do support it,
the API requires processes to be started as root (CAP_NET_RAW) which
is not desirable from a general security perspective.
This patch set leverages Daniel Mack's work to attach bpf programs to
a cgroup:
https://www.mail-archive.com/netdev@vger.kernel.org/msg134028.html
to provide a capability to set sk_bound_dev_if for all AF_INET{6}
sockets opened by a process in a cgroup when the sockets are allocated.
This capability enables running any program in a VRF context and is key
to deploying Management VRF, a fundamental configuration for networking
gear, with any Linux OS installation.
David Ahern (3):
bpf: Refactor cgroups code in prep for new type
bpf: Add new cgroups prog type to enable sock modifications
samples: bpf: add userspace example for modifying sk_bound_dev_if
include/linux/filter.h | 2 +-
include/uapi/linux/bpf.h | 15 +++++++
kernel/bpf/cgroup.c | 36 ++++++++++++++---
kernel/bpf/syscall.c | 32 +++++++++------
net/core/filter.c | 92 +++++++++++++++++++++++++++++++++++++++++++
net/core/sock.c | 7 ++++
samples/bpf/Makefile | 2 +
samples/bpf/bpf_helpers.h | 2 +
samples/bpf/test_cgrp2_sock.c | 84 +++++++++++++++++++++++++++++++++++++++
9 files changed, 253 insertions(+), 19 deletions(-)
create mode 100644 samples/bpf/test_cgrp2_sock.c
--
2.1.4
^ permalink raw reply
* [PATCH net] inet: Fix missing return value in inet6_hash
From: Craig Gallek @ 2016-10-25 22:08 UTC (permalink / raw)
To: David Miller; +Cc: Soheil Hassas Yeganeh, netdev
From: Craig Gallek <kraig@google.com>
As part of a series to implement faster SO_REUSEPORT lookups,
commit 086c653f5862 ("sock: struct proto hash function may error")
added return values to protocol hash functions and
commit 496611d7b5ea ("inet: create IPv6-equivalent inet_hash function")
implemented a new hash function for IPv6. However, the latter does
not respect the former's convention.
This properly propagates the hash errors in the IPv6 case.
Fixes: 496611d7b5ea ("inet: create IPv6-equivalent inet_hash function")
Reported-by: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: Craig Gallek <kraig@google.com>
---
net/ipv6/inet6_hashtables.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 2fd0374a35b1..02761c9fe43e 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -264,13 +264,15 @@ EXPORT_SYMBOL_GPL(inet6_hash_connect);
int inet6_hash(struct sock *sk)
{
+ int err = 0;
+
if (sk->sk_state != TCP_CLOSE) {
local_bh_disable();
- __inet_hash(sk, NULL, ipv6_rcv_saddr_equal);
+ err = __inet_hash(sk, NULL, ipv6_rcv_saddr_equal);
local_bh_enable();
}
- return 0;
+ return err;
}
EXPORT_SYMBOL_GPL(inet6_hash);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [net-next PATCH 04/27] arch/arc: Add option to skip sync on DMA mapping
From: Vineet Gupta @ 2016-10-25 22:00 UTC (permalink / raw)
To: Alexander Duyck, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Cc: Vineet Gupta, linux-snps-arc@lists.infradead.org,
davem@davemloft.net, brouer@redhat.com
In-Reply-To: <20161025153709.4815.82720.stgit@ahduyck-blue-test.jf.intel.com>
On 10/25/2016 02:38 PM, Alexander Duyck wrote:
> This change allows us to pass DMA_ATTR_SKIP_CPU_SYNC which allows us to
> avoid invoking cache line invalidation if the driver will just handle it
> later via a sync_for_cpu or sync_for_device call.
>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: linux-snps-arc@lists.infradead.org
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> arch/arc/mm/dma.c | 5 ++++-
Acked-by: Vineet Gupta <vgupta@synopsys.com>
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
> index 20afc65..6303c34 100644
> --- a/arch/arc/mm/dma.c
> +++ b/arch/arc/mm/dma.c
> @@ -133,7 +133,10 @@ static dma_addr_t arc_dma_map_page(struct device *dev, struct page *page,
> unsigned long attrs)
> {
> phys_addr_t paddr = page_to_phys(page) + offset;
> - _dma_cache_sync(paddr, size, dir);
> +
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> + _dma_cache_sync(paddr, size, dir);
> +
> return plat_phys_to_dma(dev, paddr);
> }
>
>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [RFC PATCH ethtool 2/2] ethtool: Support for FEC encoding control
From: Vidya Sagar Ravipati @ 2016-10-25 21:39 UTC (permalink / raw)
To: davem, netdev, linville, saeedm, galp, odedw, ariela, tzahio,
ddecotig
Cc: roees, dustin, roopa, aviadr
From: Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
As FEC settings and different FEC modes are mandatory
and configurable across various interfaces of 25G/50G/100G/40G ,
the lack of FEC encoding control and reporting today is a source
for interoperability issues for many vendors
set-fec/show-fec option(s) are designed to provide control and report
the FEC encoding on the link.
root@tor: ethtool --set-fec swp1 encoding [off | RS | BaseR | auto] autoneg [off | on]
Encoding: Types of encoding
Off : Turning off any encoding
RS : enforcing RS-FEC encoding on supported speeds
BaseR : enforcing Base R encoding on supported speeds
Auto : Default FEC settings for divers , and would represent
asking the hardware to essentially go into a best effort mode.
Here are a few examples of what we would expect if encoding=auto:
- if autoneg is on, we are expecting FEC to be negotiated as on or off
as long as protocol supports it
- if the hardware is capable of detecting the FEC encoding on it's
receiver it will reconfigure its encoder to match
- in absence of the above, the configuration would be set to IEEE
defaults.
>From our understanding , this is essentially what most hardware/driver
combinations are doing today in the absence of a way for users to
control the behavior.
root@tor: ethtool --show-fec swp1
FEC parameters for swp1:
Autonegotiate: off
FEC encodings: RS
ethtool devname output:
root@tor:~# ethtool swp1
Settings for swp1:
root@hpe-7712-03:~# ethtool swp18
Settings for swp18:
Supported ports: [ FIBRE ]
Supported link modes: 40000baseCR4/Full
40000baseSR4/Full
40000baseLR4/Full
100000baseSR4/Full
100000baseCR4/Full
100000baseLR4_ER4/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Supported FEC modes: [RS | BaseR | None | Not reported]
Advertised link modes: Not reported
Advertised pause frame use: No
Advertised auto-negotiation: No
Advertised FEC modes: [RS | BaseR | None | Not reported]
Speed: 100000Mb/s
Duplex: Full
Port: FIBRE
PHYAD: 106
Transceiver: internal
Auto-negotiation: off
Link detected: yes
Signed-off-by: Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
---
ethtool.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 152 insertions(+)
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..7fa058c 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -684,6 +684,7 @@ static void dump_link_caps(const char *prefix, const char *an_prefix,
};
int indent;
int did1, new_line_pend, i;
+ int fecreported = 0;
/* Indent just like the separate functions used to */
indent = strlen(prefix) + 14;
@@ -735,6 +736,26 @@ static void dump_link_caps(const char *prefix, const char *an_prefix,
fprintf(stdout, "Yes\n");
else
fprintf(stdout, "No\n");
+
+ fprintf(stdout, " %s FEC modes: ", prefix);
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_FEC_NONE_BIT, mask)) {
+ fprintf(stdout, "None\n");
+ fecreported = 1;
+ }
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_FEC_BASER_BIT, mask)) {
+ fprintf(stdout, "BaseR\n");
+ fecreported = 1;
+ }
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_FEC_RS_BIT, mask)) {
+ fprintf(stdout, "RS\n");
+ fecreported = 1;
+ }
+ if (!fecreported) {
+ fprintf(stdout, "Not reported\n");
+ }
}
}
@@ -1562,6 +1583,42 @@ static void dump_eeecmd(struct ethtool_eee *ep)
dump_link_caps("Link partner advertised EEE", "", link_mode, 1);
}
+static void dump_feccmd(struct ethtool_fecparam *ep)
+{
+ static char buf[300];
+
+ memset(buf, 0, sizeof(buf));
+
+ bool first = true;
+
+ fprintf(stdout,
+ "Auto-negotiation: %s\n",
+ ep->autoneg ? "on" : "off");
+ fprintf(stdout, "FEC encodings :");
+
+ if(ep->fec & ETHTOOL_FEC_NONE) {
+ strcat(buf, "NotSupported");
+ first = false;
+ }
+ if(ep->fec & ETHTOOL_FEC_OFF) {
+ strcat(buf, "None");
+ first = false;
+ }
+ if(ep->fec & ETHTOOL_FEC_BASER) {
+ if (!first)
+ strcat(buf, " | ");
+ strcat(buf, "BaseR");
+ first = false;
+ }
+ if(ep->fec & ETHTOOL_FEC_RS) {
+ if (!first)
+ strcat(buf, " | ");
+ strcat(buf, "RS");
+ first = false;
+ }
+ fprintf(stdout," %s\n", buf);
+}
+
#define N_SOTS 7
static char *so_timestamping_labels[N_SOTS] = {
@@ -4520,6 +4577,97 @@ static int do_seee(struct cmd_context *ctx)
return 0;
}
+static int fecmode_str_to_type(const char *str)
+{
+ int fecmode = 0;
+
+ if (str == NULL)
+ return fecmode;
+
+ if (!strcmp(str, "auto"))
+ fecmode |= ETHTOOL_FEC_AUTO;
+ else if (!strcmp(str, "off"))
+ fecmode |= ETHTOOL_FEC_OFF;
+ else if (!strcmp(str, "rs"))
+ fecmode |= ETHTOOL_FEC_RS;
+ else if (!strcmp(str, "baser"))
+ fecmode |= ETHTOOL_FEC_BASER;
+
+ return fecmode;
+}
+
+static int do_gfec(struct cmd_context *ctx)
+{
+ struct ethtool_fecparam feccmd;
+
+ if (ctx->argc != 0)
+ exit_bad_args();
+
+ fprintf(stdout, "FEC parameters for %s:\n", ctx->devname);
+
+ feccmd.cmd = ETHTOOL_GFECPARAM;
+ if (send_ioctl(ctx, &feccmd)) {
+ perror("Cannot get FEC settings");
+ return 1;
+ }
+
+ dump_feccmd(&feccmd);
+ return 0;
+}
+
+static int do_sfec(struct cmd_context *ctx)
+{
+ int fec_changed = -1;
+ int changed = 1;
+ struct ethtool_fecparam feccmd;
+ int autoneg_val = -1;
+ int fecmode;
+ char *fecmode_str = NULL;
+ struct cmdline_info cmdline_fec[] = {
+ { "autoneg", CMDL_BOOL, &autoneg_val,
+ &feccmd.autoneg },
+ { "encoding", CMDL_STR, &fecmode_str, &feccmd.fec},
+ };
+
+ parse_generic_cmdline(ctx, &fec_changed,
+ cmdline_fec, ARRAY_SIZE(cmdline_fec));
+
+ if(fecmode_str == NULL)
+ exit_bad_args();
+
+ fecmode = fecmode_str_to_type(fecmode_str);
+ if (!fecmode)
+ exit_bad_args();
+
+ /* Get current FEC parameters */
+ feccmd.cmd = ETHTOOL_GFECPARAM;
+ if (send_ioctl(ctx, &feccmd)) {
+ perror("Cannot get FEC settings");
+ return 1;
+ }
+
+ /* Compare autoneg and fec parameter and if they are same, reject it */
+ if (feccmd.fec == fecmode) {
+ if (feccmd.autoneg == autoneg_val) {
+ changed = 0;
+ }
+ }
+
+ if (!changed) {
+ fprintf(stderr, "no fec parameters changed, aborting\n");
+ return 93;
+ }
+
+ feccmd.cmd = ETHTOOL_SFECPARAM;
+ feccmd.fec = fecmode;
+ feccmd.autoneg = autoneg_val;
+ if (send_ioctl(ctx, &feccmd)) {
+ perror("Cannot set FEC settings");
+ return 1;
+ }
+ return 0;
+}
+
#ifndef TEST_ETHTOOL
int send_ioctl(struct cmd_context *ctx, void *cmd)
{
@@ -4681,6 +4829,10 @@ static const struct option {
" [ advertise %x ]\n"
" [ tx-lpi on|off ]\n"
" [ tx-timer %d ]\n"},
+ { "--show-fec", 1, do_gfec, "Show FEC settings"},
+ { "--set-fec", 1, do_sfec, "Set FEC settings",
+ " [ autoneg on|off ]\n"
+ " [ encoding on|off|RS|BaseR ]\n"},
{ "-h|--help", 0, show_usage, "Show this help" },
{ "--version", 0, do_version, "Show version number" },
{}
--
2.1.4
^ permalink raw reply related
* [RFC PATCH ethtool 1/2] ethtool-copy.h: sync with net
From: Vidya Sagar Ravipati @ 2016-10-25 21:39 UTC (permalink / raw)
To: davem, netdev, linville, saeedm, galp, odedw, ariela, tzahio,
ddecotig
Cc: roees, dustin, roopa, aviadr
From: Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
Sending out this review as RFC to get early feedback
on fec options and output changes as changes to kernel uapi
to ethtool is under review on netdev currently and might
change based on review..
http://patchwork.ozlabs.org/patch/686293/
Signed-off-by: Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
---
ethtool-copy.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 51 insertions(+), 2 deletions(-)
diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..ff3f4f0 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -1222,6 +1222,51 @@ struct ethtool_per_queue_op {
char data[];
};
+/**
+ * struct ethtool_fecparam - Ethernet forward error correction(fec) parameters
+ * @cmd: Command number = %ETHTOOL_GFECPARAM or %ETHTOOL_SFECPARAM
+ * @autoneg: Flag to enable autonegotiation of fec modes(rs,baser)
+ * (D44:47 of base link code word)
+ * @fec: Bitmask of supported FEC modes
+ * @rsvd: Reserved for future extensions. i.e FEC bypass feature.
+ *
+ * Drivers should reject a non-zero setting of @autoneg when
+ * autoneogotiation is disabled (or not supported) for the link.
+ *
+ * If @autoneg is non-zero, the MAC is configured to enable one of
+ * the supported FEC modes according to the result of autonegotiation.
+ * Otherwise, it is configured directly based on the @fec parameter
+ */
+struct ethtool_fecparam {
+ __u32 cmd;
+ __u32 autoneg;
+ /* bitmask of FEC modes */
+ __u32 fec;
+ __u32 reserved;
+};
+
+/**
+ * enum ethtool_fec_config_bits - flags definition of ethtool_fec_configuration
+ * @ETHTOOL_FEC_NONE: FEC mode configuration is not supported
+ * @ETHTOOL_FEC_AUTO: Default/Best FEC mode provided by driver
+ * @ETHTOOL_FEC_OFF: No FEC Mode
+ * @ETHTOOL_FEC_RS: Reed-Solomon Forward Error Detection mode
+ * @ETHTOOL_FEC_BASER: Base-R/Reed-Solomon Forward Error Detection mode
+ */
+enum ethtool_fec_config_bits {
+ ETHTOOL_FEC_NONE_BIT,
+ ETHTOOL_FEC_AUTO_BIT,
+ ETHTOOL_FEC_OFF_BIT,
+ ETHTOOL_FEC_RS_BIT,
+ ETHTOOL_FEC_BASER_BIT,
+};
+
+#define ETHTOOL_FEC_NONE (1 << ETHTOOL_FEC_NONE_BIT)
+#define ETHTOOL_FEC_AUTO (1 << ETHTOOL_FEC_AUTO_BIT)
+#define ETHTOOL_FEC_OFF (1 << ETHTOOL_FEC_OFF_BIT)
+#define ETHTOOL_FEC_RS (1 << ETHTOOL_FEC_RS_BIT)
+#define ETHTOOL_FEC_BASER (1 << ETHTOOL_FEC_BASER_BIT)
+
/* CMDs currently supported */
#define ETHTOOL_GSET 0x00000001 /* DEPRECATED, Get settings.
* Please use ETHTOOL_GLINKSETTINGS
@@ -1313,6 +1358,8 @@ struct ethtool_per_queue_op {
#define ETHTOOL_GLINKSETTINGS 0x0000004c /* Get ethtool_link_settings */
#define ETHTOOL_SLINKSETTINGS 0x0000004d /* Set ethtool_link_settings */
+#define ETHTOOL_GFECPARAM 0x0000004e /* Get FEC settings */
+#define ETHTOOL_SFECPARAM 0x0000004f /* Set FEC settings */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
@@ -1367,7 +1414,9 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_10000baseLR_Full_BIT = 44,
ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT = 45,
ETHTOOL_LINK_MODE_10000baseER_Full_BIT = 46,
-
+ ETHTOOL_LINK_MODE_FEC_NONE_BIT = 47,
+ ETHTOOL_LINK_MODE_FEC_RS_BIT = 48,
+ ETHTOOL_LINK_MODE_FEC_BASER_BIT = 49,
/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
* 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
@@ -1376,7 +1425,7 @@ enum ethtool_link_mode_bit_indices {
*/
__ETHTOOL_LINK_MODE_LAST
- = ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+ = ETHTOOL_LINK_MODE_FEC_BASER_BIT,
};
#define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name) \
--
2.1.4
^ permalink raw reply related
* [RFC PATCH ethtool 0/2] ethtool: Add support for FEC encoding configuration
From: Vidya Sagar Ravipati @ 2016-10-25 21:39 UTC (permalink / raw)
To: davem, netdev, linville, saeedm, galp, odedw, ariela, tzahio,
ddecotig
Cc: roees, dustin, roopa, aviadr
From: Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
Forward Error Correction (FEC) modes i.e Base-R
and Reed-Solomon modes are introduced in 25G/40G/100G standards
for providing good BER at high speeds.
Various networking devices which support 25G/40G/100G provides ability
to manage supported FEC modes and the lack of FEC encoding control and
reporting today is a source for itneroperability issues for many vendors.
FEC capability as well as specific FEC mode i.e. Base-R
or RS modes can be requested or advertised through bits D44:47 of base link
codeword.
This patch set intends to provide option under ethtool to manage and report
FEC encoding settings for networking devices as per IEEE 802.3 bj, bm and by
specs.
set-fec/show-fec option(s) are designed to provide control and report
the FEC encoding on the link.
SET FEC option:
root@tor: ethtool --set-fec swp1 encoding [off | RS | BaseR | auto] autoneg [off | on]
Encoding: Types of encoding
Off : Turning off any encoding
RS : enforcing RS-FEC encoding on supported speeds
BaseR : enforcing Base R encoding on supported speeds
Auto : Default FEC settings for divers , and would represent
asking the hardware to essentially go into a best effort mode.
Here are a few examples of what we would expect if encoding=auto:
- if autoneg is on, we are expecting FEC to be negotiated as on or off
as long as protocol supports it
- if the hardware is capable of detecting the FEC encoding on it's
receiver it will reconfigure its encoder to match
- in absence of the above, the configuration would be set to IEEE
defaults.
>From our understanding , this is essentially what most hardware/driver
combinations are doing today in the absence of a way for users to
control the behavior.
SHOW FEC option:
root@tor: ethtool --show-fec swp1
FEC parameters for swp1:
Autonegotiate: off
FEC encodings: RS
ETHTOOL DEVNAME output modification:
ethtool devname output:
root@tor:~# ethtool swp1
Settings for swp1:
root@hpe-7712-03:~# ethtool swp18
Settings for swp18:
Supported ports: [ FIBRE ]
Supported link modes: 40000baseCR4/Full
40000baseSR4/Full
40000baseLR4/Full
100000baseSR4/Full
100000baseCR4/Full
100000baseLR4_ER4/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Supported FEC modes: [RS | BaseR | None | Not reported]
Advertised link modes: Not reported
Advertised pause frame use: No
Advertised auto-negotiation: No
Advertised FEC modes: [RS | BaseR | None | Not reported]
<<<< One or more FEC modes
Speed: 100000Mb/s
Duplex: Full
Port: FIBRE
PHYAD: 106
Transceiver: internal
Auto-negotiation: off
Link detected: yes
Vidya Sagar Ravipati (2):
ethtool-copy.h: sync with net
ethtool: Support for FEC encoding control
ethtool-copy.h | 53 +++++++++++++++++++-
ethtool.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 203 insertions(+), 2 deletions(-)
--
2.1.4
^ permalink raw reply
* Re: [PATCH net-next] ibmveth: calculate correct gso_size and set gso_type
From: Jonathan Maxwell @ 2016-10-25 21:20 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: tlfalcon, benh, paulus, mpe, David Miller, Tom Herbert, jarod,
hofrat, netdev, linuxppc-dev, linux-kernel, Jon Maxwell
In-Reply-To: <20161025103126.GW2948@localhost.localdomain>
>> + u16 hdr_len = ETH_HLEN + sizeof(struct tcphdr);
> Compiler may optmize this, but maybe move hdr_len to [*] ?>
There are other places in the stack where a u16 is used for the
same purpose. So I'll rather stick to that convention.
I'll make the other formatting changes you suggested and
resubmit as v1.
Thanks
Jon
On Tue, Oct 25, 2016 at 9:31 PM, Marcelo Ricardo Leitner
<mleitner@redhat.com> wrote:
> On Tue, Oct 25, 2016 at 04:13:41PM +1100, Jon Maxwell wrote:
>> We recently encountered a bug where a few customers using ibmveth on the
>> same LPAR hit an issue where a TCP session hung when large receive was
>> enabled. Closer analysis revealed that the session was stuck because the
>> one side was advertising a zero window repeatedly.
>>
>> We narrowed this down to the fact the ibmveth driver did not set gso_size
>> which is translated by TCP into the MSS later up the stack. The MSS is
>> used to calculate the TCP window size and as that was abnormally large,
>> it was calculating a zero window, even although the sockets receive buffer
>> was completely empty.
>>
>> We were able to reproduce this and worked with IBM to fix this. Thanks Tom
>> and Marcelo for all your help and review on this.
>>
>> The patch fixes both our internal reproduction tests and our customers tests.
>>
>> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
>> ---
>> drivers/net/ethernet/ibm/ibmveth.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
>> index 29c05d0..3028c33 100644
>> --- a/drivers/net/ethernet/ibm/ibmveth.c
>> +++ b/drivers/net/ethernet/ibm/ibmveth.c
>> @@ -1182,6 +1182,8 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
>> int frames_processed = 0;
>> unsigned long lpar_rc;
>> struct iphdr *iph;
>> + bool large_packet = 0;
>> + u16 hdr_len = ETH_HLEN + sizeof(struct tcphdr);
>
> Compiler may optmize this, but maybe move hdr_len to [*] ?
>
>>
>> restart_poll:
>> while (frames_processed < budget) {
>> @@ -1236,10 +1238,27 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
>> iph->check = 0;
>> iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
>> adapter->rx_large_packets++;
>> + large_packet = 1;
>> }
>> }
>> }
>>
>> + if (skb->len > netdev->mtu) {
>
> [*]
>
>> + iph = (struct iphdr *)skb->data;
>> + if (be16_to_cpu(skb->protocol) == ETH_P_IP && iph->protocol == IPPROTO_TCP) {
>
> The if line above is too long, should be broken in two.
>
>> + hdr_len += sizeof(struct iphdr);
>> + skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>> + skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
>> + } else if (be16_to_cpu(skb->protocol) == ETH_P_IPV6 &&
>> + iph->protocol == IPPROTO_TCP) {
> ^
> And this one should start 3 spaces later, right below be16_....
>
> Marcelo
>
>> + hdr_len += sizeof(struct ipv6hdr);
>> + skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>> + skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
>> + }
>> + if (!large_packet)
>> + adapter->rx_large_packets++;
>> + }
>> +
>> napi_gro_receive(napi, skb); /* send it up */
>>
>> netdev->stats.rx_packets++;
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* [PATCH v2] cw1200: fix bogus maybe-uninitialized warning
From: Arnd Bergmann @ 2016-10-25 20:21 UTC (permalink / raw)
To: Solomon Peachy, Kalle Valo
Cc: David Laight, Arnd Bergmann, Johannes Berg, linux-wireless,
netdev, linux-kernel
On x86, the cw1200 driver produces a rather silly warning about the
possible use of the 'ret' variable without an initialization
presumably after being confused by the architecture specific definition
of WARN_ON:
drivers/net/wireless/st/cw1200/wsm.c: In function ‘wsm_handle_rx’:
drivers/net/wireless/st/cw1200/wsm.c:1457:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
We have already checked that 'count' is larger than 0 here, so
we know that 'ret' is initialized. Changing the 'for' loop
into do/while also makes this clear to the compiler.
Suggested-by: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/st/cw1200/wsm.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
v2: rewrite based on David Laight's suggestion, the first version
was completely wrong.
diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
index 680d60eabc75..ed93bf3474ec 100644
--- a/drivers/net/wireless/st/cw1200/wsm.c
+++ b/drivers/net/wireless/st/cw1200/wsm.c
@@ -379,7 +379,6 @@ static int wsm_multi_tx_confirm(struct cw1200_common *priv,
{
int ret;
int count;
- int i;
count = WSM_GET32(buf);
if (WARN_ON(count <= 0))
@@ -395,11 +394,10 @@ static int wsm_multi_tx_confirm(struct cw1200_common *priv,
}
cw1200_debug_txed_multi(priv, count);
- for (i = 0; i < count; ++i) {
+ do {
ret = wsm_tx_confirm(priv, buf, link_id);
- if (ret)
- return ret;
- }
+ } while (!ret && --count);
+
return ret;
underflow:
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] cw1200: fix bogus maybe-uninitialized warning
From: Arnd Bergmann @ 2016-10-25 20:19 UTC (permalink / raw)
To: David Laight
Cc: Solomon Peachy, Kalle Valo, Johannes Berg,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0209E9A@AcuExch.aculab.com>
On Tuesday, October 25, 2016 1:24:55 PM CEST David Laight wrote:
> > diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
> > index 680d60eabc75..094e6637ade2 100644
> > --- a/drivers/net/wireless/st/cw1200/wsm.c
> > +++ b/drivers/net/wireless/st/cw1200/wsm.c
> > @@ -385,14 +385,13 @@ static int wsm_multi_tx_confirm(struct cw1200_common *priv,
> > if (WARN_ON(count <= 0))
> > return -EINVAL;
> >
> > - if (count > 1) {
> > - /* We already released one buffer, now for the rest */
> > - ret = wsm_release_tx_buffer(priv, count - 1);
> > - if (ret < 0)
> > - return ret;
> > - else if (ret > 0)
> > - cw1200_bh_wakeup(priv);
> > - }
> > + /* We already released one buffer, now for the rest */
> > + ret = wsm_release_tx_buffer(priv, count - 1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (ret > 0)
> > + cw1200_bh_wakeup(priv);
>
> That doesn't look equivalent to me (when count == 1).
Ah, that's what I missed, thanks for pointing that out!
> >
> > cw1200_debug_txed_multi(priv, count);
> > for (i = 0; i < count; ++i) {
>
> Convert this loop into a do ... while so the body executes at least once.
Good idea. Version 2 coming now.
Arnd
^ permalink raw reply
* Re: [PATCH] virtio-net: Update the mtu code to match virtio spec
From: Aaron Conole @ 2016-10-25 20:14 UTC (permalink / raw)
To: netdev; +Cc: virtualization, linux-kernel, Michael S. Tsirkin, Jarod Wilson
In-Reply-To: <f7ta8ds9ory.fsf@redhat.com>
Aaron Conole <aconole@redhat.com> writes:
>> From: Aaron Conole <aconole@bytheb.org>
>>
>> The virtio committee recently ratified a change, VIRTIO-152, which
>> defines the mtu field to be 'max' MTU, not simply desired MTU.
>>
>> This commit brings the virtio-net device in compliance with VIRTIO-152.
>>
>> Additionally, drop the max_mtu branch - it cannot be taken since the u16
>> returned by virtio_cread16 will never exceed the initial value of
>> max_mtu.
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Jarod Wilson <jarod@redhat.com>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>
> Sorry about the subject line, David. This is targetted at net-next, and
> it appears my from was mangled. Would you like me to resubmit with
> these details corrected?
I answered my own question. Sorry for the noise.
^ permalink raw reply
* [PATCH v2 net-next] virtio-net: Update the mtu code to match virtio spec
From: Aaron Conole @ 2016-10-25 20:12 UTC (permalink / raw)
To: netdev; +Cc: Jarod Wilson, Michael S. Tsirkin, linux-kernel, virtualization
The virtio committee recently ratified a change, VIRTIO-152, which
defines the mtu field to be 'max' MTU, not simply desired MTU.
This commit brings the virtio-net device in compliance with VIRTIO-152.
Additionally, drop the max_mtu branch - it cannot be taken since the u16
returned by virtio_cread16 will never exceed the initial value of
max_mtu.
Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: "Michael S. Tsirkin" <mst@redhat.com>
Acked-by: Jarod Wilson <jarod@redhat.com>
---
Nothing code-wise has changed, but I've included the ACKs and fixed up the
subject line.
drivers/net/virtio_net.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 720809f..2cafd12 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1870,10 +1870,12 @@ static int virtnet_probe(struct virtio_device *vdev)
mtu = virtio_cread16(vdev,
offsetof(struct virtio_net_config,
mtu));
- if (mtu < dev->min_mtu || mtu > dev->max_mtu)
+ if (mtu < dev->min_mtu) {
__virtio_clear_bit(vdev, VIRTIO_NET_F_MTU);
- else
+ } else {
dev->mtu = mtu;
+ dev->max_mtu = mtu;
+ }
}
if (vi->any_header_sg)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] virtio-net: Update the mtu code to match virtio spec
From: Aaron Conole @ 2016-10-25 20:06 UTC (permalink / raw)
To: netdev; +Cc: virtualization, linux-kernel, Michael S. Tsirkin, Jarod Wilson
In-Reply-To: <1477413335-17296-1-git-send-email-aconole@redhat.com>
> From: Aaron Conole <aconole@bytheb.org>
>
> The virtio committee recently ratified a change, VIRTIO-152, which
> defines the mtu field to be 'max' MTU, not simply desired MTU.
>
> This commit brings the virtio-net device in compliance with VIRTIO-152.
>
> Additionally, drop the max_mtu branch - it cannot be taken since the u16
> returned by virtio_cread16 will never exceed the initial value of
> max_mtu.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jarod Wilson <jarod@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
Sorry about the subject line, David. This is targetted at net-next, and
it appears my from was mangled. Would you like me to resubmit with
these details corrected?
-Aaron
^ 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