Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 nf-next 2/2] netfilter: x_tables: don't use seqlock when fetching old counters
From: Florian Westphal @ 2017-10-10 21:39 UTC (permalink / raw)
  To: netdev, edumazet; +Cc: Florian Westphal, Dan Williams
In-Reply-To: <20171010213945.19074-1-fw@strlen.de>

after previous commit xt_replace_table will wait until all cpus
had even seqcount (i.e., no cpu is accessing old ruleset).

Add a 'old' counter retrival version that doesn't synchronize counters.
Its not needed, the old counters are not in use anymore at this point.

This speeds up table replacement on busy systems with large tables
(and many cores).

Cc: Dan Williams <dcbw@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: fix Erics email address

 net/ipv4/netfilter/arp_tables.c | 22 ++++++++++++++++++++--
 net/ipv4/netfilter/ip_tables.c  | 23 +++++++++++++++++++++--
 net/ipv6/netfilter/ip6_tables.c | 22 ++++++++++++++++++++--
 3 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 9e2770fd00be..f88221aebc9d 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -634,6 +634,25 @@ static void get_counters(const struct xt_table_info *t,
 	}
 }
 
+static void get_old_counters(const struct xt_table_info *t,
+			     struct xt_counters counters[])
+{
+	struct arpt_entry *iter;
+	unsigned int cpu, i;
+
+	for_each_possible_cpu(cpu) {
+		i = 0;
+		xt_entry_foreach(iter, t->entries, t->size) {
+			struct xt_counters *tmp;
+
+			tmp = xt_get_per_cpu_counter(&iter->counters, cpu);
+			ADD_COUNTER(counters[i], tmp->bcnt, tmp->pcnt);
+			++i;
+		}
+		cond_resched();
+	}
+}
+
 static struct xt_counters *alloc_counters(const struct xt_table *table)
 {
 	unsigned int countersize;
@@ -910,8 +929,7 @@ static int __do_replace(struct net *net, const char *name,
 	    (newinfo->number <= oldinfo->initial_entries))
 		module_put(t->me);
 
-	/* Get the old counters, and synchronize with replace */
-	get_counters(oldinfo, counters);
+	get_old_counters(oldinfo, counters);
 
 	/* Decrease module usage counts and free resource */
 	loc_cpu_old_entry = oldinfo->entries;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 39286e543ee6..4cbe5e80f3bf 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -781,6 +781,26 @@ get_counters(const struct xt_table_info *t,
 	}
 }
 
+static void get_old_counters(const struct xt_table_info *t,
+			     struct xt_counters counters[])
+{
+	struct ipt_entry *iter;
+	unsigned int cpu, i;
+
+	for_each_possible_cpu(cpu) {
+		i = 0;
+		xt_entry_foreach(iter, t->entries, t->size) {
+			const struct xt_counters *tmp;
+
+			tmp = xt_get_per_cpu_counter(&iter->counters, cpu);
+			ADD_COUNTER(counters[i], tmp->bcnt, tmp->pcnt);
+			++i; /* macro does multi eval of i */
+		}
+
+		cond_resched();
+	}
+}
+
 static struct xt_counters *alloc_counters(const struct xt_table *table)
 {
 	unsigned int countersize;
@@ -1070,8 +1090,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 	    (newinfo->number <= oldinfo->initial_entries))
 		module_put(t->me);
 
-	/* Get the old counters, and synchronize with replace */
-	get_counters(oldinfo, counters);
+	get_old_counters(oldinfo, counters);
 
 	/* Decrease module usage counts and free resource */
 	xt_entry_foreach(iter, oldinfo->entries, oldinfo->size)
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 01bd3ee5ebc6..f06e25065a34 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -800,6 +800,25 @@ get_counters(const struct xt_table_info *t,
 	}
 }
 
+static void get_old_counters(const struct xt_table_info *t,
+			     struct xt_counters counters[])
+{
+	struct ip6t_entry *iter;
+	unsigned int cpu, i;
+
+	for_each_possible_cpu(cpu) {
+		i = 0;
+		xt_entry_foreach(iter, t->entries, t->size) {
+			const struct xt_counters *tmp;
+
+			tmp = xt_get_per_cpu_counter(&iter->counters, cpu);
+			ADD_COUNTER(counters[i], tmp->bcnt, tmp->pcnt);
+			++i;
+		}
+		cond_resched();
+	}
+}
+
 static struct xt_counters *alloc_counters(const struct xt_table *table)
 {
 	unsigned int countersize;
@@ -1090,8 +1109,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 	    (newinfo->number <= oldinfo->initial_entries))
 		module_put(t->me);
 
-	/* Get the old counters, and synchronize with replace */
-	get_counters(oldinfo, counters);
+	get_old_counters(oldinfo, counters);
 
 	/* Decrease module usage counts and free resource */
 	xt_entry_foreach(iter, oldinfo->entries, oldinfo->size)
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 nf-next 1/2] netfilter: x_tables: make xt_replace_table wait until old rules are not used anymore
From: Florian Westphal @ 2017-10-10 21:39 UTC (permalink / raw)
  To: netdev, edumazet; +Cc: Florian Westphal, Dan Williams
In-Reply-To: <20171010213945.19074-1-fw@strlen.de>

xt_replace_table relies on table replacement counter retrieval (which
uses xt_recseq to synchronize pcpu counters).

This is fine, however with large rule set get_counters() can take
a very long time -- it needs to synchronize all counters because
it has to assume concurrent modifications can occur.

Make xt_replace_table synchronize by itself by waiting until all cpus
had an even seqcount.

This allows a followup patch to copy the counters of the old ruleset
without any synchonization after xt_replace_table has completed.

Cc: Dan Williams <dcbw@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: fix Erics email address

 net/netfilter/x_tables.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index c83a3b5e1c6c..f2d4a365768f 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1153,6 +1153,7 @@ xt_replace_table(struct xt_table *table,
 	      int *error)
 {
 	struct xt_table_info *private;
+	unsigned int cpu;
 	int ret;
 
 	ret = xt_jumpstack_alloc(newinfo);
@@ -1184,12 +1185,20 @@ xt_replace_table(struct xt_table *table,
 
 	/*
 	 * Even though table entries have now been swapped, other CPU's
-	 * may still be using the old entries. This is okay, because
-	 * resynchronization happens because of the locking done
-	 * during the get_counters() routine.
+	 * may still be using the old entries...
 	 */
 	local_bh_enable();
 
+	/* ... so wait for even xt_recseq on all cpus */
+	for_each_possible_cpu(cpu) {
+		seqcount_t *s = &per_cpu(xt_recseq, cpu);
+
+		while (raw_read_seqcount(s) & 1)
+			cpu_relax();
+
+		cond_resched();
+	}
+
 #ifdef CONFIG_AUDIT
 	if (audit_enabled) {
 		audit_log(current->audit_context, GFP_KERNEL,
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 nf-next] netfilter: x_tables: speed up iptables-restore
From: Florian Westphal @ 2017-10-10 21:39 UTC (permalink / raw)
  To: netdev, edumazet

iptables-restore can take quite a long time when sytem is busy,
in order of half a minute or more.
The main reason for this is the way ip(6)tables performs table
swap, or, more precisely, expensive sequence lock synchronizations
when reading counters.

When xt_replace_table assigns the new ruleset pointer, it does
not wait for other processors to finish with old ruleset.

Instead it relies on the counter sequence lock in get_counters()
to do this.

This works but this is very costly if system is busy as each counter
read operation can possibly be restarted indefinitely.

Instead, make xt_replace_table wait until all processors are
known to not use the old ruleset anymore.

This allows to read the old counters without any locking, no cpu is
using the ruleset anymore so counters can't change either.

 ipv4/netfilter/arp_tables.c |   22 ++++++++++++++++++++--
 ipv4/netfilter/ip_tables.c  |   23 +++++++++++++++++++++--
 ipv6/netfilter/ip6_tables.c |   22 ++++++++++++++++++++--
 netfilter/x_tables.c        |   15 ++++++++++++---
 4 files changed, 73 insertions(+), 9 deletions(-)

^ permalink raw reply

* Re: [PATCH v2] xdp: Sample xdp program implementing ip forward
From: David Daney @ 2017-10-10 21:37 UTC (permalink / raw)
  To: Stephen Hemminger, Christina Jacob
  Cc: Christina Jacob, daniel, Sunil.Goutham, netdev, dsahern,
	linux-kernel, brouer, linux-arm-kernel
In-Reply-To: <20171010101921.30136d48@shemminger-XPS-13-9360>

On 10/10/2017 10:19 AM, Stephen Hemminger wrote:
> On Tue, 10 Oct 2017 12:58:52 +0530
> Christina Jacob <christina.jacob.koikara@gmail.com> wrote:
> 
>> +/* Get the mac address of the interface given interface name */
>> +static long *getmac(char *iface)
>> +{
>> +	int fd;
>> +	struct ifreq ifr;
>> +	long *mac = NULL;
>> +
>> +	fd = socket(AF_INET, SOCK_DGRAM, 0);
>> +	ifr.ifr_addr.sa_family = AF_INET;
>> +	strncpy(ifr.ifr_name, iface, IFNAMSIZ - 1);
>> +	ioctl(fd, SIOCGIFHWADDR, &ifr);
>> +	mac = (long *)ifr.ifr_hwaddr.sa_data;
>> +	close(fd);
>> +	return mac;
> 
> Always check return value of ioctl.
> You are assuming sizeof(long) > 6 bytes.
> Also the byte order.


Also:

Returning the address of a local variable (ifr.ifr_hwaddr.sa_data), and 
then dereferencing it outside of the function is not correct.

The casting of the char sa_data[] to a long * may cause alignment faults 
on some architectures.  The may also be endinaness issues depending on 
how the data are manipulated if you pack all those chars into a long.

If we think that a MAC address is char[6], then it may be best to define 
the data structures as such and manipulate it as an array instead of 
trying to pack it into a long.

Keep working on this though, this program will surely be useful.

David Daney

^ permalink raw reply

* Re: [Patch net-next] tcp: add a tracepoint for tcp_retransmit_skb()
From: Cong Wang @ 2017-10-10 21:37 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Linux Kernel Network Developers, Eric Dumazet, Yuchung Cheng,
	Neal Cardwell, Martin KaFai Lau, Brendan Gregg,
	Hannes Frederic Sowa, Song Liu
In-Reply-To: <20171010173821.6djxyvrggvaivqec@ast-mbp.dhcp.thefacebook.com>

On Tue, Oct 10, 2017 at 10:38 AM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> I'm happy to see new tracepoints being added to tcp stack, but I'm concerned
> with practical usability of them.
> Like the above tracepoint definition makes it not very useful from bpf point of view,
> since 'sk' pointer is not recored by as part of the tracepoint.
> In bpf/tracing world we prefer tracepoints to have raw pointers recorded
> in TP_STRUCT__entry() and _not_ printed in TP_printk()
> (since pointers are useless for userspace).
> Like trace_kfree_skb() tracepoint records raw 'skb' pointer and we can
> walk whatever sk_buff fields we need inside the program.
> Such approach allows tracepoint to be usable in many more scenarios, since
> bpf program can examine kernel datastructures.


Sure, I am happy to add them for BPF. The current version is merely
for our own use case, other use cases like this are always welcome!


> Over the last few years we've been running tcp statistics framework (similar to web10g)
> using 8 kprobes in tcp stack with bpf programs extracting the data and now we're
> ready to share this experience with the community. Right now we're working on a set
> of tracepoints for tcp stack to make the interface more accurate, faster and more stable.
> We're planning to send an RFC patch with these new tracepoints in the comming weeks.

Great! Looking forward to it!

>
> More concrete, if you can make this trace_tcp_retransmit_skb() to record
> sk, skb pointers and err code at the end of __tcp_retransmit_skb() it will solve
> our need as well.


Note, currently I only call trace_tcp_retransmit_skb() for successful
retransmissions, since you mentioned err code, I guess you want it
for failures too? I am not sure if tracing unsuccessful TCP retransmissions
is meaningful here, I guess it's needed for BPF to track TCP states?

It doesn't harm to add it, at least we can filter out err!=0 since we
only care about successful ones.


>
> So far our list of kprobes is:
> int kprobe__tcp_validate_incoming
> int kprobe__tcp_send_active_reset
> int kprobe__tcp_v4_send_reset
> int kprobe__tcp_v6_send_reset
> int kprobe__tcp_v4_destroy_sock
> int kprobe__tcp_set_state
> int kprobe__tcp_retransmit_skb
> int kprobe__tcp_rtx_synack
>
> with tracepoints we can consolidate two of them into one and drop
> another one for sure. Notice that tcp_retransmit_skb is on our list too
> and currently we're doing extra work inside the program to make it more
> accurate which will be unnecessary if this tracepoint is at the end
> of __tcp_retransmit_skb().

Yeah, with these tracepoints we would be able to trace more TCP
state changes.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next v2 6/7] bpf: don't rely on the verifier lock for metadata_dst allocation
From: kbuild test robot @ 2017-10-10 21:33 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: kbuild-all, netdev, oss-drivers, alexei.starovoitov, daniel,
	Jakub Kicinski
In-Reply-To: <20171009173015.23520-7-jakub.kicinski@netronome.com>

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

Hi Jakub,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jakub-Kicinski/bpf-get-rid-of-global-verifier-state-and-reuse-instruction-printer/20171011-021905
config: x86_64-randconfig-a0-10110234 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   net//core/dst.c: In function 'metadata_dst_free_percpu':
>> net//core/dst.c:328: warning: unused variable 'cpu'

vim +/cpu +328 net//core/dst.c

   325	
   326	void metadata_dst_free_percpu(struct metadata_dst __percpu *md_dst)
   327	{
 > 328		int cpu;
   329	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30274 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v2 4/5] selinux: bpf: Add selinux check for eBPF syscall operations
From: kbuild test robot @ 2017-10-10 21:30 UTC (permalink / raw)
  To: Chenbo Feng
  Cc: kbuild-all, linux-security-module, netdev, SELinux,
	Jeffrey Vander Stoep, Alexei Starovoitov, lorenzo,
	Daniel Borkmann, Stephen Smalley, Chenbo Feng
In-Reply-To: <20171009222028.13096-5-chenbofeng.kernel@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

Hi Chenbo,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Chenbo-Feng/bpf-security-New-file-mode-and-LSM-hooks-for-eBPF-object-permission-control/20171011-010349
config: x86_64-randconfig-u0-10110310 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/init.h:4:0,
                    from security/selinux/hooks.c:27:
   security/selinux/hooks.c: In function 'bpf_map_fmode_to_av':
   security/selinux/hooks.c:6284:6: error: 'f_mode' undeclared (first use in this function)
     if (f_mode & FMODE_READ)
         ^
   include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> security/selinux/hooks.c:6284:2: note: in expansion of macro 'if'
     if (f_mode & FMODE_READ)
     ^~
   security/selinux/hooks.c:6284:6: note: each undeclared identifier is reported only once for each function it appears in
     if (f_mode & FMODE_READ)
         ^
   include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> security/selinux/hooks.c:6284:2: note: in expansion of macro 'if'
     if (f_mode & FMODE_READ)
     ^~

vim +/if +6284 security/selinux/hooks.c

  6279	
  6280	static u32 bpf_map_fmode_to_av(fmode_t fmode)
  6281	{
  6282		u32 av = 0;
  6283	
> 6284		if (f_mode & FMODE_READ)
  6285			av |= BPF_MAP__READ;
  6286		if (f_mode & FMODE_WRITE)
  6287			av |= BPF_MAP__WRITE;
  6288		return av;
  6289	}
  6290	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31549 bytes --]

^ permalink raw reply

* Re: [patch net-next 1/4] net: sched: make tc_action_ops->get_dev return dev and avoid passing net
From: Jiri Pirko @ 2017-10-10 21:19 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Saeed Mahameed, matanb, leonro, mlxsw
In-Reply-To: <CAM_iQpXjq1QVVAC2L3TdTEJzdda1=H1V=c4n5v+qeYJqCSxoWw@mail.gmail.com>

Tue, Oct 10, 2017 at 07:44:53PM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Oct 10, 2017 at 12:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> -static int tcf_mirred_device(const struct tc_action *a, struct net *net,
>> -                            struct net_device **mirred_dev)
>> +static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
>>  {
>> -       int ifindex = tcf_mirred_ifindex(a);
>> +       struct tcf_mirred *m = to_mirred(a);
>>
>> -       *mirred_dev = __dev_get_by_index(net, ifindex);
>> -       if (!*mirred_dev)
>> -               return -EINVAL;
>> -       return 0;
>> +       return __dev_get_by_index(m->net, m->tcfm_ifindex);
>
>Hmm, why not just return m->tcfm_dev?

I just follow the existing code. The change you suggest should be a
separate follow-up patch.

^ permalink raw reply

* Re: [patch net-next 3/4] net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev infra
From: Jiri Pirko @ 2017-10-10 21:16 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Simon Horman, Linux Netdev List, David Miller, Jamal Hadi Salim,
	Cong Wang, Saeed Mahameed, mlxsw
In-Reply-To: <CAJ3xEMh0cM3FszEjKNNe3aWTjHoDxof9GURgUSMy5k3iSTLg2g@mail.gmail.com>

Tue, Oct 10, 2017 at 10:08:23PM CEST, gerlitz.or@gmail.com wrote:
>On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> The only user of cls_flower->egress_dev is mlx5.
>
>but nfp supports decap action offload too and from the flower code
>stand point, I guess they are both the same, right? how does it work
>there?

Apparently they don't use cls_flower->egress_dev.

^ permalink raw reply

* Re: [patch net-next 0/4] net: sched: get rid of cls_flower->egress_dev
From: Jiri Pirko @ 2017-10-10 21:13 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Saeed Mahameed, Linux Netdev List, David Miller, Jamal Hadi Salim,
	Cong Wang, mlxsw
In-Reply-To: <CAJ3xEMgPdcVrogHZmEnH+vP3E53zvEcFN8+wyuEqSs6utHQVRg@mail.gmail.com>

Tue, Oct 10, 2017 at 07:24:21PM CEST, gerlitz.or@gmail.com wrote:
>Jiri,
>
>FWIW, as I reported to you earlier, I was playing with tc encap/decap rules
>on 4.14-rc+ (net) before
>applying any patch of this series, and something is messy w.r.t to decap
>rules. I don't see
>them removed at all when user space attempts to do so. It might (probably)
>mlx5 bug, which
>we will have to fix for net and later rebase net-next over net. We have
>short WW here so
>I will not be able to do RCA this week.

Or, as I replied to you earlier, the issue you describe is totally
unrelated to this patchset as you see the issue with the current net-next.
Not sure why you add this comment here.

^ permalink raw reply

* [PATCH net-next v2] openvswitch: add ct_clear action
From: Eric Garver @ 2017-10-10 20:54 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA; +Cc: dev-yBygre7rU0TnMu66kgdUjQ

This adds a ct_clear action for clearing conntrack state. ct_clear is
currently implemented in OVS userspace, but is not backed by an action
in the kernel datapath. This is useful for flows that may modify a
packet tuple after a ct lookup has already occurred.

Signed-off-by: Eric Garver <e@erig.me>
---
v2:
	- Use IP_CT_UNTRACKED for nf_ct_set()
	- Only fill key if previously conntracked

 include/uapi/linux/openvswitch.h |  2 ++
 net/openvswitch/actions.c        |  4 ++++
 net/openvswitch/conntrack.c      | 11 +++++++++++
 net/openvswitch/conntrack.h      |  7 +++++++
 net/openvswitch/flow_netlink.c   |  5 +++++
 5 files changed, 29 insertions(+)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index efdbfbfd3ee2..0cd6f8833147 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -807,6 +807,7 @@ struct ovs_action_push_eth {
  * packet.
  * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
  * packet.
+ * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
  *
  * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all
  * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
@@ -836,6 +837,7 @@ enum ovs_action_attr {
 	OVS_ACTION_ATTR_TRUNC,        /* u32 struct ovs_action_trunc. */
 	OVS_ACTION_ATTR_PUSH_ETH,     /* struct ovs_action_push_eth. */
 	OVS_ACTION_ATTR_POP_ETH,      /* No argument. */
+	OVS_ACTION_ATTR_CT_CLEAR,     /* No argument. */
 
 	__OVS_ACTION_ATTR_MAX,	      /* Nothing past this will be accepted
 				       * from userspace. */
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index a54a556fcdb5..a551232daf61 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1203,6 +1203,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 				return err == -EINPROGRESS ? 0 : err;
 			break;
 
+		case OVS_ACTION_ATTR_CT_CLEAR:
+			err = ovs_ct_clear(skb, key);
+			break;
+
 		case OVS_ACTION_ATTR_PUSH_ETH:
 			err = push_eth(skb, key, nla_data(a));
 			break;
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index d558e882ca0c..fe861e2f0deb 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1129,6 +1129,17 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
 	return err;
 }
 
+int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key)
+{
+	if (skb_nfct(skb)) {
+		nf_conntrack_put(skb_nfct(skb));
+		nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
+		ovs_ct_fill_key(skb, key);
+	}
+
+	return 0;
+}
+
 static int ovs_ct_add_helper(struct ovs_conntrack_info *info, const char *name,
 			     const struct sw_flow_key *key, bool log)
 {
diff --git a/net/openvswitch/conntrack.h b/net/openvswitch/conntrack.h
index bc7efd1867ab..399dfdd2c4f9 100644
--- a/net/openvswitch/conntrack.h
+++ b/net/openvswitch/conntrack.h
@@ -30,6 +30,7 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *, struct sk_buff *);
 
 int ovs_ct_execute(struct net *, struct sk_buff *, struct sw_flow_key *,
 		   const struct ovs_conntrack_info *);
+int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key);
 
 void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key);
 int ovs_ct_put_key(const struct sw_flow_key *swkey,
@@ -73,6 +74,12 @@ static inline int ovs_ct_execute(struct net *net, struct sk_buff *skb,
 	return -ENOTSUPP;
 }
 
+static inline int ovs_ct_clear(struct sk_buff *skb,
+			       struct sw_flow_key *key)
+{
+	return -ENOTSUPP;
+}
+
 static inline void ovs_ct_fill_key(const struct sk_buff *skb,
 				   struct sw_flow_key *key)
 {
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index fc0ca9a89b8e..dc0d79092e74 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -76,6 +76,7 @@ static bool actions_may_change_flow(const struct nlattr *actions)
 			break;
 
 		case OVS_ACTION_ATTR_CT:
+		case OVS_ACTION_ATTR_CT_CLEAR:
 		case OVS_ACTION_ATTR_HASH:
 		case OVS_ACTION_ATTR_POP_ETH:
 		case OVS_ACTION_ATTR_POP_MPLS:
@@ -2528,6 +2529,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
 			[OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
 			[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash),
 			[OVS_ACTION_ATTR_CT] = (u32)-1,
+			[OVS_ACTION_ATTR_CT_CLEAR] = 0,
 			[OVS_ACTION_ATTR_TRUNC] = sizeof(struct ovs_action_trunc),
 			[OVS_ACTION_ATTR_PUSH_ETH] = sizeof(struct ovs_action_push_eth),
 			[OVS_ACTION_ATTR_POP_ETH] = 0,
@@ -2669,6 +2671,9 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
 			skip_copy = true;
 			break;
 
+		case OVS_ACTION_ATTR_CT_CLEAR:
+			break;
+
 		case OVS_ACTION_ATTR_PUSH_ETH:
 			/* Disallow pushing an Ethernet header if one
 			 * is already present */
-- 
2.12.0

^ permalink raw reply related

* Re: [ovs-dev] [PATCH net-next] openvswitch: add ct_clear action
From: Joe Stringer @ 2017-10-10 20:24 UTC (permalink / raw)
  To: Eric Garver, Joe Stringer, Pravin Shelar,
	Linux Kernel Network Developers, ovs dev
In-Reply-To: <20171010191329.GH26353@dev-rhel7>

On 10 October 2017 at 12:13, Eric Garver <e@erig.me> wrote:
> On Tue, Oct 10, 2017 at 10:24:20AM -0700, Joe Stringer wrote:
>> On 10 October 2017 at 08:09, Eric Garver <e@erig.me> wrote:
>> > On Tue, Oct 10, 2017 at 05:33:48AM -0700, Joe Stringer wrote:
>> >> On 9 October 2017 at 21:41, Pravin Shelar <pshelar@ovn.org> wrote:
>> >> > On Fri, Oct 6, 2017 at 9:44 AM, Eric Garver <e@erig.me> wrote:
>> >> >> This adds a ct_clear action for clearing conntrack state. ct_clear is
>> >> >> currently implemented in OVS userspace, but is not backed by an action
>> >> >> in the kernel datapath. This is useful for flows that may modify a
>> >> >> packet tuple after a ct lookup has already occurred.
>> >> >>
>> >> >> Signed-off-by: Eric Garver <e@erig.me>
>> >> > Patch mostly looks good. I have following comments.
>> >> >
>> >> >> ---
>> >> >>  include/uapi/linux/openvswitch.h |  2 ++
>> >> >>  net/openvswitch/actions.c        |  5 +++++
>> >> >>  net/openvswitch/conntrack.c      | 12 ++++++++++++
>> >> >>  net/openvswitch/conntrack.h      |  7 +++++++
>> >> >>  net/openvswitch/flow_netlink.c   |  5 +++++
>> >> >>  5 files changed, 31 insertions(+)
>> >> >>
>> >> >> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
>> >> >> index 156ee4cab82e..1b6e510e2cc6 100644
>> >> >> --- a/include/uapi/linux/openvswitch.h
>> >> >> +++ b/include/uapi/linux/openvswitch.h
>> >> >> @@ -806,6 +806,7 @@ struct ovs_action_push_eth {
>> >> >>   * packet.
>> >> >>   * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
>> >> >>   * packet.
>> >> >> + * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
>> >> >>   *
>> >> >>   * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all
>> >> >>   * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
>> >> >> @@ -835,6 +836,7 @@ enum ovs_action_attr {
>> >> >>         OVS_ACTION_ATTR_TRUNC,        /* u32 struct ovs_action_trunc. */
>> >> >>         OVS_ACTION_ATTR_PUSH_ETH,     /* struct ovs_action_push_eth. */
>> >> >>         OVS_ACTION_ATTR_POP_ETH,      /* No argument. */
>> >> >> +       OVS_ACTION_ATTR_CT_CLEAR,     /* No argument. */
>> >> >>
>> >> >>         __OVS_ACTION_ATTR_MAX,        /* Nothing past this will be accepted
>> >> >>                                        * from userspace. */
>> >> >> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> >> >> index a54a556fcdb5..db9c7f2e662b 100644
>> >> >> --- a/net/openvswitch/actions.c
>> >> >> +++ b/net/openvswitch/actions.c
>> >> >> @@ -1203,6 +1203,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> >>                                 return err == -EINPROGRESS ? 0 : err;
>> >> >>                         break;
>> >> >>
>> >> >> +               case OVS_ACTION_ATTR_CT_CLEAR:
>> >> >> +                       err = ovs_ct_clear(skb, key);
>> >> >> +                       break;
>> >> >> +
>> >> >>                 case OVS_ACTION_ATTR_PUSH_ETH:
>> >> >>                         err = push_eth(skb, key, nla_data(a));
>> >> >>                         break;
>> >> >> @@ -1210,6 +1214,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> >>                 case OVS_ACTION_ATTR_POP_ETH:
>> >> >>                         err = pop_eth(skb, key);
>> >> >>                         break;
>> >> >> +
>> >> >>                 }
>> >> > Unrelated change.
>> >> >
>> >> >>
>> >> >>                 if (unlikely(err)) {
>> >> >> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
>> >> >> index d558e882ca0c..f9b73c726ad7 100644
>> >> >> --- a/net/openvswitch/conntrack.c
>> >> >> +++ b/net/openvswitch/conntrack.c
>> >> >> @@ -1129,6 +1129,18 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
>> >> >>         return err;
>> >> >>  }
>> >> >>
>> >> >> +int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key)
>> >> >> +{
>> >> >> +       if (skb_nfct(skb)) {
>> >> >> +               nf_conntrack_put(skb_nfct(skb));
>> >> >> +               nf_ct_set(skb, NULL, 0);
>> >> > Can the new conntract state be appropriate? may be IP_CT_UNTRACKED?
>> >> >
>> >> >> +       }
>> >> >> +
>> >> >> +       ovs_ct_fill_key(skb, key);
>> >> >> +
>> >> > I do not see need to refill the key if there is no skb-nf-ct.
>> >>
>> >> Really this is trying to just zero the CT key fields, but reuses
>> >> existing functions, right? This means that subsequent upcalls, for
>> >
>> > Right.
>> >
>> >> instance, won't have the outdated view of the CT state from the
>> >> previous lookup (that was prior to the ct_clear). I'd expect these key
>> >> fields to be cleared.
>> >
>> > I assumed Pravin was saying that we don't need to clear them if there is
>> > no conntrack state. They should already be zero.
>>
>> The conntrack calls aren't going to clear it, so I don't see what else
>> would clear it?
>>
>> If you execute ct(),ct_clear(), then the first ct will set the
>> values.. what will zero them?
>
> I meant move ovs_ct_fill_key() to inside the if statement.
> i.e.
>
>        if (skb_nfct(skb)) {
>                nf_conntrack_put(skb_nfct(skb));
>                nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
>                ovs_ct_fill_key(skb, key);
>        }
>
> Should be nothing to fill/zero if we have not yet done conntrack.
> Is there a case where we may lose skb->_nfct, but the key still has
> conntrack data?

Ah, misreading on my part. Right, if there is no nfct then it should
already have the right value. I don't think there's a way to lose it
and leave the key with conntrack data.

^ permalink raw reply

* Re: [net-next 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2017-10-10
From: David Miller @ 2017-10-10 20:21 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Oct 2017 10:21:30 -0700

> This series contains updates to e1000e and igb.

Pulled, thanks Jeff.

^ permalink raw reply

* Re: [PATCH net 0/2] nfp: fix ethtool stats and page allocation
From: David Miller @ 2017-10-10 20:18 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20171010161623.23838-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 10 Oct 2017 09:16:21 -0700

> Two fixes for net.  First one makes sure we handle gather of stats on
> 32bit machines correctly (ouch).  The second fix solves a potential
> NULL-deref if we fail to allocate a page with XDP running.
> 
> I used Fixes: tags pointing to where the bug was introduced, but for
> patch 1 it has been in the driver "for ever" and fix won't backport
> cleanly beyond commit 325945ede6d4 ("nfp: split software and hardware 
> vNIC statistics") which is in net.

Series applied, thanks Jakub.

^ permalink raw reply

* Re: [net 0/2][pull request] Intel Wired LAN Driver Updates 2017-10-10
From: David Miller @ 2017-10-10 20:17 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20171010151416.43149-1-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Oct 2017 08:14:14 -0700

> This series contains updates to i40e only.

Pulled, thanks Jeff.

^ permalink raw reply

* Re: [PATCH v4 net-next] rtnetlink: bridge: use ext_ack instead of printk
From: David Miller @ 2017-10-10 20:16 UTC (permalink / raw)
  To: fw; +Cc: netdev, dsahern
In-Reply-To: <20171010151004.20056-1-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Tue, 10 Oct 2017 17:10:04 +0200

> We can now piggyback error strings to userspace via extended acks
> rather than using printk.
> 
> Before:
> bridge fdb add 01:02:03:04:05:06 dev br0 vlan 4095
> RTNETLINK answers: Invalid argument
> 
> After:
> bridge fdb add 01:02:03:04:05:06 dev br0 vlan 4095
> Error: invalid vlan id.
> 
> v3: drop 'RTM_' prefixes, suggested by David Ahern, they
> are not useful, the add/del in bridge command line is enough.
> 
> Also reword error in response to malformed/bad vlan id attribute
> size.
> 
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] selftests: rtnetlink: test RTM_GETNETCONF
From: David Miller @ 2017-10-10 20:15 UTC (permalink / raw)
  To: fw; +Cc: netdev
In-Reply-To: <20171010141805.19194-1-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Tue, 10 Oct 2017 16:18:05 +0200

> exercise RTM_GETNETCONF call path for unspec, inet and inet6
> families, they are DOIT_UNLOCKED candidates.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH net-next 0/3] mlx4_en num of rings
From: David Miller @ 2017-10-10 20:11 UTC (permalink / raw)
  To: tariqt; +Cc: netdev, eranbe
In-Reply-To: <1507627715-25487-1-git-send-email-tariqt@mellanox.com>

From: Tariq Toukan <tariqt@mellanox.com>
Date: Tue, 10 Oct 2017 12:28:32 +0300

> This patchset from Inbar contains changes to rings control
> to the mlx4 Eth driver.
> 
> Patches 1 and 2 limit the number of rings to the number of CPUs.
> Patch 3 removes a limitation in logic of default number of RX rings.
> 
> Series generated against net-next commit:
> 812b5ca7d376 Add a driver for Renesas uPD60620 and uPD60620A PHYs

Series applied, thanks Tariq.

^ permalink raw reply

* RE: [PATCH] i40e: mark PM functions as __maybe_unused
From: Keller, Jacob E @ 2017-10-10 20:11 UTC (permalink / raw)
  To: Arnd Bergmann, Kirsher, Jeffrey T
  Cc: Williams, Mitch A, Duyck, Alexander H, David S. Miller,
	Brady, Alan, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20171010081847.4090496-1-arnd@arndb.de>



> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, October 10, 2017 1:18 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>; Keller, Jacob E
> <jacob.e.keller@intel.com>; Williams, Mitch A <mitch.a.williams@intel.com>;
> Duyck, Alexander H <alexander.h.duyck@intel.com>; David S. Miller
> <davem@davemloft.net>; Brady, Alan <alan.brady@intel.com>; intel-wired-
> lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] i40e: mark PM functions as __maybe_unused
> 
> A cleanup of the PM code left an incorrect #ifdef in place, leading
> to a harmless build warning:
> 
> drivers/net/ethernet/intel/i40e/i40e_main.c:12223:12: error: 'i40e_resume'
> defined but not used [-Werror=unused-function]
> drivers/net/ethernet/intel/i40e/i40e_main.c:12185:12: error: 'i40e_suspend'
> defined but not used [-Werror=unused-function]
> 
> It's easier to use __maybe_unused attributes here, since you
> can't pick the wrong one.
> 

Sure.

Acked-by: Jacob Keller <jacob.e.keller@intel.com>

> Fixes: 0e5d3da40055 ("i40e: use newer generic PM support instead of legacy PM
> callbacks")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 60b11fdeca2d..eb091268bc3c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -8370,7 +8370,6 @@ static int i40e_init_interrupt_scheme(struct i40e_pf
> *pf)
>  	return 0;
>  }
> 
> -#ifdef CONFIG_PM
>  /**
>   * i40e_restore_interrupt_scheme - Restore the interrupt scheme
>   * @pf: private board data structure
> @@ -8419,7 +8418,6 @@ static int i40e_restore_interrupt_scheme(struct i40e_pf
> *pf)
> 
>  	return err;
>  }
> -#endif /* CONFIG_PM */
> 
>  /**
>   * i40e_setup_misc_vector - Setup the misc vector to handle non queue events
> @@ -12177,12 +12175,11 @@ static void i40e_shutdown(struct pci_dev *pdev)
>  	}
>  }
> 
> -#ifdef CONFIG_PM
>  /**
>   * i40e_suspend - PM callback for moving to D3
>   * @dev: generic device information structure
>   **/
> -static int i40e_suspend(struct device *dev)
> +static int __maybe_unused i40e_suspend(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct i40e_pf *pf = pci_get_drvdata(pdev);
> @@ -12220,7 +12217,7 @@ static int i40e_suspend(struct device *dev)
>   * i40e_resume - PM callback for waking up from D3
>   * @dev: generic device information structure
>   **/
> -static int i40e_resume(struct device *dev)
> +static int __maybe_unused i40e_resume(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct i40e_pf *pf = pci_get_drvdata(pdev);
> @@ -12252,8 +12249,6 @@ static int i40e_resume(struct device *dev)
>  	return 0;
>  }
> 
> -#endif /* CONFIG_PM */
> -
>  static const struct pci_error_handlers i40e_err_handler = {
>  	.error_detected = i40e_pci_error_detected,
>  	.slot_reset = i40e_pci_error_slot_reset,
> @@ -12269,11 +12264,9 @@ static struct pci_driver i40e_driver = {
>  	.id_table = i40e_pci_tbl,
>  	.probe    = i40e_probe,
>  	.remove   = i40e_remove,
> -#ifdef CONFIG_PM
>  	.driver   = {
>  		.pm = &i40e_pm_ops,
>  	},
> -#endif /* CONFIG_PM */
>  	.shutdown = i40e_shutdown,
>  	.err_handler = &i40e_err_handler,
>  	.sriov_configure = i40e_pci_sriov_configure,
> --
> 2.9.0

^ permalink raw reply

* Re: [PATCH net-next 0/5] Support set_ringparam and {set|get}_rxnfc ethtool commands
From: David Miller @ 2017-10-10 20:09 UTC (permalink / raw)
  To: lipeng321; +Cc: netdev, linux-kernel, linuxarm, yisen.zhuang, salil.mehta
In-Reply-To: <1507624927-98008-1-git-send-email-lipeng321@huawei.com>

From: Lipeng <lipeng321@huawei.com>
Date: Tue, 10 Oct 2017 16:42:02 +0800

> 1, Patch [1/5,2/5] add support for ethtool ops set_ringparam
>    (ethtool -G) and fix related bug.
> 2, Patch [3/5,4/5, 5/5] add support for ethtool ops
>    set_rxnfc/get_rxnfc (-n/-N) and fix related bug. 

Series applied, thank you.

^ permalink raw reply

* [PATCH net-next 1/1] veth: tweak creation of veth device
From: Roman Mashak @ 2017-10-10 20:08 UTC (permalink / raw)
  To: davem; +Cc: jhs, netdev, Roman Mashak

When creating veth pair, at first rtnl_new_link() creates veth_dev, i.e.
one end of the veth pipe, but not registers it; then veth_newlink() gets
invoked, where peer dev is created _and_ registered, followed by veth_dev
registration, which may fail if peer information, that is VETH_INFO_PEER
attribute, has not been provided and the kernel will allocate unique veth
name.

So, we should ask the kernel to allocate unique name for veth_dev only
when peer info is not available.

Example:

% ip link dev veth0 type veth
RTNETLINK answers: File exists

After fix:
% ip link dev veth0 type veth
% ip link show dev veth0
5: veth0@veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether f6:ef:8b:96:f4:ec brd ff:ff:ff:ff:ff:ff
%

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 drivers/net/veth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index f5438d0..00dce15 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -432,7 +432,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 	if (tb[IFLA_ADDRESS] == NULL)
 		eth_hw_addr_random(dev);
 
-	if (tb[IFLA_IFNAME])
+	if (ifmp && tb[IFLA_IFNAME])
 		nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
-- 
1.9.1

^ permalink raw reply related

* Re: [patch net-next 3/4] net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev infra
From: Or Gerlitz @ 2017-10-10 20:08 UTC (permalink / raw)
  To: Jiri Pirko, Simon Horman
  Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
	Saeed Mahameed, mlxsw
In-Reply-To: <20171010073016.3682-4-jiri@resnulli.us>

On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> The only user of cls_flower->egress_dev is mlx5.

but nfp supports decap action offload too and from the flower code
stand point, I guess they are both the same, right? how does it work
there?

Or.

^ permalink raw reply

* Re: [patch net-next 3/4] net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev infra
From: Or Gerlitz @ 2017-10-10 20:04 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
	Saeed Mahameed, Matan Barak, Leon Romanovsky, mlxsw
In-Reply-To: <20171010073016.3682-4-jiri@resnulli.us>

On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:

> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -206,8 +206,6 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts);
>  int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);
>  int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
>                      struct net_device **hw_dev);
> -int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
> -                          void *type_data, bool err_stop);

but this (and another 1-2 hunks below) were set by upstream patch of
this series, did you do that add/del on purpose? is that for
bisection? if not why?

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
From: Florian Fainelli @ 2017-10-10 19:55 UTC (permalink / raw)
  To: Jes Sorensen, Gustavo A. R. Silva, Kalle Valo
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5f5f0f54-d901-90be-9025-0a1c4b909368-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 10/10/2017 12:35 PM, Jes Sorensen wrote:
> On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> where we are expecting to fall through.
> 
> While this isn't harmful, to me this looks like pointless patch churn
> for zero gain and it's just ugly.

That is the canonical way to tell static analyzers and compilers that
fall throughs are wanted and not accidental mistakes in the code. For
people that deal with these kinds of errors, it's quite helpful, unless
you suggest disabling that particular GCC warning specific for that
file/directory?

> 
> Jes
> 
> 
>> Cc: Jes Sorensen <Jes.Sorensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Signed-off-by: Gustavo A. R. Silva <garsilva-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
>> ---
>>   drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 7806a4d..e66be05 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct
>> ieee80211_hw *hw)
>>       switch (hw->conf.chandef.width) {
>>       case NL80211_CHAN_WIDTH_20_NOHT:
>>           ht = false;
>> +        /* fall through */
>>       case NL80211_CHAN_WIDTH_20:
>>           opmode |= BW_OPMODE_20MHZ;
>>           rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
>> @@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct
>> ieee80211_hw *hw)
>>       switch (hw->conf.chandef.width) {
>>       case NL80211_CHAN_WIDTH_20_NOHT:
>>           ht = false;
>> +        /* fall through */
>>       case NL80211_CHAN_WIDTH_20:
>>           rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
>>           subchannel = 0;
>> @@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct
>> rtl8xxxu_priv *priv)
>>           case 3:
>>               priv->ep_tx_low_queue = 1;
>>               priv->ep_tx_count++;
>> +            /* fall through */
>>           case 2:
>>               priv->ep_tx_normal_queue = 1;
>>               priv->ep_tx_count++;
>> +            /* fall through */
>>           case 1:
>>               priv->ep_tx_high_queue = 1;
>>               priv->ep_tx_count++;
>> @@ -5691,6 +5695,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw
>> *hw, enum set_key_cmd cmd,
>>           break;
>>       case WLAN_CIPHER_SUITE_TKIP:
>>           key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
>> +        /* fall through */
>>       default:
>>           return -EOPNOTSUPP;
>>       }
>>
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: add new T5 pci device id's
From: David Miller @ 2017-10-10 19:52 UTC (permalink / raw)
  To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh
In-Reply-To: <1507619702-8793-1-git-send-email-ganeshgr@chelsio.com>

From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Tue, 10 Oct 2017 12:45:02 +0530

> Add 0x50aa and 0x50ab T5 device id's.
> 
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>

Applied.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox