Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] RDS: IB: Initialize max_items based on underlying device attributes
From: Santosh Shilimkar @ 2017-10-04  1:07 UTC (permalink / raw)
  To: Avinash Repaka, linux-rdma; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <1507078237-32366-1-git-send-email-avinash.repaka@oracle.com>

Hi Avinash,

On 10/3/2017 5:50 PM, Avinash Repaka wrote:
> Use max_1m_mrs/max_8k_mrs while setting max_items, as the former
> variables are set based on the underlying device attricutes.
> 
s/attricutes/attributes

> Signed-off-by: Avinash Repaka <avinash.repaka@oracle.com>
> ---
>   net/rds/ib_rdma.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
This patch is fine but it will be nice if you can collect
these changes in series as you are trying to update the
FRWR support. Like this patch and other cleanup patch
you posted yesterday.

With that log fixed,
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

^ permalink raw reply

* [net-next:master 146/148] htmldocs: net/core/dev.c:1306: warning: No description found for parameter 'name'
From: kbuild test robot @ 2017-10-04  1:56 UTC (permalink / raw)
  To: Florian Westphal; +Cc: kbuild-all, netdev

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head:   26873308b21654b6e0785b9f9e2c5414d37a4c4c
commit: 6c5570016b972d9b1f0f6c2dca9cc0422b1f92bf [146/148] net: core: decouple ifalias get/set from rtnl lock
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
   kernel/trace/blktrace.c:818: warning: No description found for parameter 'cgid'
   include/linux/sync_file.h:51: warning: No description found for parameter 'flags'
   drivers/gpio/gpiolib.c:593: warning: No description found for parameter '16'
   drivers/gpio/gpiolib.c:593: warning: Excess struct/union/enum/typedef member 'events' description in 'lineevent_state'
   include/linux/usb/gadget.h:232: warning: No description found for parameter 'claimed'
   include/linux/usb/gadget.h:232: warning: No description found for parameter 'enabled'
   include/linux/usb/gadget.h:414: warning: No description found for parameter 'quirk_altset_not_supp'
   include/linux/usb/gadget.h:414: warning: No description found for parameter 'quirk_stall_not_supp'
   include/linux/usb/gadget.h:414: warning: No description found for parameter 'quirk_zlp_not_supp'
   fs/inode.c:1680: warning: No description found for parameter 'rcu'
   include/linux/jbd2.h:443: warning: No description found for parameter 'i_transaction'
   include/linux/jbd2.h:443: warning: No description found for parameter 'i_next_transaction'
   include/linux/jbd2.h:443: warning: No description found for parameter 'i_list'
   include/linux/jbd2.h:443: warning: No description found for parameter 'i_vfs_inode'
   include/linux/jbd2.h:443: warning: No description found for parameter 'i_flags'
   include/linux/jbd2.h:497: warning: No description found for parameter 'h_rsv_handle'
   include/linux/jbd2.h:497: warning: No description found for parameter 'h_reserved'
   include/linux/jbd2.h:497: warning: No description found for parameter 'h_type'
   include/linux/jbd2.h:497: warning: No description found for parameter 'h_line_no'
   include/linux/jbd2.h:497: warning: No description found for parameter 'h_start_jiffies'
   include/linux/jbd2.h:497: warning: No description found for parameter 'h_requested_credits'
   include/linux/jbd2.h:497: warning: No description found for parameter 'saved_alloc_context'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chkpt_bhs'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_devname'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_average_commit_time'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_min_batch_time'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_max_batch_time'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_commit_callback'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_failed_commit'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chksum_driver'
   include/linux/jbd2.h:1050: warning: No description found for parameter 'j_csum_seed'
   fs/jbd2/transaction.c:511: warning: No description found for parameter 'type'
   fs/jbd2/transaction.c:511: warning: No description found for parameter 'line_no'
   fs/jbd2/transaction.c:641: warning: No description found for parameter 'gfp_mask'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_pin'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_unpin'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_res_obj'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_get_sg_table'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_import_sg_table'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_vmap'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_vunmap'
   include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_mmap'
   include/drm/drm_mode_config.h:771: warning: No description found for parameter 'modifiers_property'
   include/drm/drm_mode_config.h:771: warning: Excess struct/union/enum/typedef member 'modifiers' description in 'drm_mode_config'
   include/drm/drm_plane.h:544: warning: No description found for parameter 'modifiers'
   include/drm/drm_plane.h:544: warning: No description found for parameter 'modifier_count'
   drivers/gpu/host1x/bus.c:50: warning: No description found for parameter 'driver'
>> net/core/dev.c:1306: warning: No description found for parameter 'name'
>> net/core/dev.c:1306: warning: Excess function parameter 'alias' description in 'dev_get_alias'
   Documentation/core-api/kernel-api.rst:354: ERROR: Error in "kernel-doc" directive:
   unknown option: "external".

vim +/name +1306 net/core/dev.c

  1294	
  1295	/**
  1296	 *	dev_get_alias - get ifalias of a device
  1297	 *	@dev: device
  1298	 *	@alias: buffer to store name of ifalias
  1299	 *	@len: size of buffer
  1300	 *
  1301	 *	get ifalias for a device.  Caller must make sure dev cannot go
  1302	 *	away,  e.g. rcu read lock or own a reference count to device.
  1303	 */
  1304	int dev_get_alias(const struct net_device *dev, char *name, size_t len)
  1305	{
> 1306		const struct dev_ifalias *alias;
  1307		int ret = 0;
  1308	
  1309		rcu_read_lock();
  1310		alias = rcu_dereference(dev->ifalias);
  1311		if (alias)
  1312			ret = snprintf(name, len, "%s", alias->ifalias);
  1313		rcu_read_unlock();
  1314	
  1315		return ret;
  1316	}
  1317	

---
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: 6761 bytes --]

^ permalink raw reply

* Re: [RFC] bpf: remove global verifier state
From: Alexei Starovoitov @ 2017-10-04  2:52 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: daniel, dsahern, netdev, oss-drivers, david.beckett
In-Reply-To: <20171004002025.28521-1-jakub.kicinski@netronome.com>

On Tue, Oct 03, 2017 at 05:20:25PM -0700, Jakub Kicinski wrote:
> The only global state protected by the verifier lock is
> the verifier log.  Move that log to struct bpf_verifier_env
> and allow verification of multiple programs in parallel.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> Like this? :)
> 
> Compile-tested, I'm rebasing things and will test shortly.
> 
>  include/linux/bpf_verifier.h |   5 +
>  kernel/bpf/verifier.c        | 520 ++++++++++++++++++++++---------------------
>  2 files changed, 276 insertions(+), 249 deletions(-)
> 
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index b8d200f60a40..598802dd1897 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -139,6 +139,11 @@ struct bpf_verifier_env {
>  	bool allow_ptr_leaks;
>  	bool seen_direct_write;
>  	struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
> +
> +	u32 log_level;
> +	u32 log_size;
> +	u32 log_len;
> +	char *log_buf;
>  };
>  
>  int bpf_analyzer(struct bpf_prog *prog, const struct bpf_ext_analyzer_ops *ops,
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 4cf9b72c59a0..450f60e6229d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -153,27 +153,21 @@ struct bpf_call_arg_meta {
>  	int access_size;
>  };
>  
> -/* verbose verifier prints what it's seeing
> - * bpf_check() is called under lock, so no race to access these global vars
> - */
> -static u32 log_level, log_size, log_len;
> -static char *log_buf;
> -
> -static DEFINE_MUTEX(bpf_verifier_lock);

yep. looks great.
Please test it and submit officially :)
The commit aafe6ae9cee3 ("bpf: dynamically allocate digest scratch buffer")
fixed the other case where we were relying on the above mutex.
The only other spot to be adjusted is to add spin_lock/mutex or DO_ONCE() to
bpf_get_skb_set_tunnel_proto() to protect md_dst init.
imo that would be it.
Daniel, anything else comes to mind?

^ permalink raw reply

* Re: [RFC] bpf: remove global verifier state
From: Eric Dumazet @ 2017-10-04  3:24 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jakub Kicinski, daniel, dsahern, netdev, oss-drivers,
	david.beckett
In-Reply-To: <20171004025226.wutfwm6hcsj4zuph@ast-mbp>

On Tue, 2017-10-03 at 19:52 -0700, Alexei Starovoitov wrote:

> yep. looks great.
> Please test it and submit officially :)
> The commit aafe6ae9cee3 ("bpf: dynamically allocate digest scratch buffer")
> fixed the other case where we were relying on the above mutex.
> The only other spot to be adjusted is to add spin_lock/mutex or DO_ONCE() to
> bpf_get_skb_set_tunnel_proto() to protect md_dst init.
> imo that would be it.
> Daniel, anything else comes to mind?


16 MB of log (unswappable kernel memory) per active checker.

We might offer a way to oom hosts.

^ permalink raw reply

* Re: [RFC] bpf: remove global verifier state
From: Alexei Starovoitov @ 2017-10-04  3:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jakub Kicinski, daniel, dsahern, netdev, oss-drivers,
	david.beckett
In-Reply-To: <1507087446.8061.37.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Oct 03, 2017 at 08:24:06PM -0700, Eric Dumazet wrote:
> On Tue, 2017-10-03 at 19:52 -0700, Alexei Starovoitov wrote:
> 
> > yep. looks great.
> > Please test it and submit officially :)
> > The commit aafe6ae9cee3 ("bpf: dynamically allocate digest scratch buffer")
> > fixed the other case where we were relying on the above mutex.
> > The only other spot to be adjusted is to add spin_lock/mutex or DO_ONCE() to
> > bpf_get_skb_set_tunnel_proto() to protect md_dst init.
> > imo that would be it.
> > Daniel, anything else comes to mind?
> 
> 
> 16 MB of log (unswappable kernel memory) per active checker.
> 
> We might offer a way to oom hosts.

right. good point!
we need to switch to continuous copy_to_user() after a page or so.
Can even do it after every vscnprintf()
but page at a time is probably faster.

^ permalink raw reply

* Re: [next-queue PATCH v3 2/4] net/sched: Fix accessing invalid dev_queue
From: Eric Dumazet @ 2017-10-04  3:47 UTC (permalink / raw)
  To: Vinicius Costa Gomes
  Cc: netdev, intel-wired-lan, Jesus Sanchez-Palencia, jhs,
	xiyou.wangcong, jiri, andre.guedes, ivan.briano, boon.leong.ong,
	richardcochran, henrik, levipearson, rodney.cummings
In-Reply-To: <20171003234435.8979-3-vinicius.gomes@intel.com>

On Tue, 2017-10-03 at 16:44 -0700, Vinicius Costa Gomes wrote:
> From: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> 
> In qdisc_alloc() the dev_queue pointer was used without any checks being
> performed. If qdisc_create() gets a null dev_queue pointer, it just
> passes it along to qdisc_alloc(), leading to a crash. That happens if a
> root qdisc implements select_queue() and returns a null dev_queue
> pointer for an "invalid handle", for example.
> 
> One way to reproduce that is:
> 
> 1) Setup mqprio
> $ tc qdisc replace dev enp3s0 parent root mqprio num_tc 3 \
>      	   map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
> 
> 2) Replace the first inner qdisc
> $ tc qdisc replace dev enp3s0 parent 8001:1 pfifo_fast
> 
> This will lead to the following crash:

When was this bug added ?

If this is a consequence of your prior patch (1/4), then this must come
before it.

No need to add a stack trace for a not existing bug.
Instead, explain in the changelog that it is a prep work.

We try to not break the tree on purpose, so that future bisection will
not hit a point where the kernel crashes.

^ permalink raw reply

* Re: [PATCH net] net: fib_rules: Fix fib_rules_ops->compare implementations to support exact match
From: Eyal Birger @ 2017-10-04  3:58 UTC (permalink / raw)
  To: David Miller
  Cc: shmulik, netdev@vger.kernel.org, mateusz.bajorski, dsa, tgraf,
	Shmulik Ladkani
In-Reply-To: <20171003.145418.677435703318514230.davem@davemloft.net>

Hi David,

On Wed, Oct 4, 2017 at 12:54 AM, David Miller <davem@davemloft.net> wrote:
> From: Shmulik Ladkani <shmulik@nsof.io>
> Date: Sat, 30 Sep 2017 11:59:09 +0300
>
>> This leads to inconsistencies, depending on order of operations, e.g.:
>
> I don't see any inconsistency.  When you insert using NLM_F_EXCL the
> insertion fails if any existing rule matches or overlaps in any way
> with the keys in the new rule.
>
> Sorry I'm not going to apply this.

The inconsistency we saw is that 0.0.0.0/0 is treated differently compared to
all other subnets - for which overlaps are not disallowed - e.g. this succeeds:

# ip ru add from 10.0.0.0/8 iif eth2 pref 33 table 33
# ip ru add from 0.0.0.0/1 iif eth2 pref 33 table 33
# ip ru add from 128.0.0.0/1 iif eth2 pref 33 table 33

Though being functionally equivalent to adding from=0.0.0.0/0.

So our understanding was that 'different subnet==different rule', similar to the
route addition behavior with NLM_F_EXCL.

Best regards,
Eyal.

^ permalink raw reply

* Re: [PATCH net] RDS: IB: Limit the scope of has_fr/has_fmr variables
From: David Miller @ 2017-10-04  4:06 UTC (permalink / raw)
  To: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA
  Cc: avinash.repaka-QHcLZuEGTsvQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d1f841a7-0c1d-9d20-ee33-6e260b5d84c4-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

From: Santosh Shilimkar <santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 3 Oct 2017 17:42:39 -0700

> Hi Dave,
> 
> On 10/2/2017 1:30 PM, Santosh Shilimkar wrote:
>> On 10/1/2017 10:56 PM, David Miller wrote:
>>> From: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> [...]
> 
>>>
>>> Actually, reverted, this breaks the build.
>>>
>>> net/rds/rdma_transport.c:38:10: fatal error: ib.h: No such file or
>>> directory
>>>   #include "ib.h"
>>>
>>> Although I can't see how in the world this patch is causing such
>>> an error.
>>>
>> Weird indeed. Will sort this out with Avinash. Thanks Dave.
>> 
> I tried few build combinations on net-next but couldn't
> reproduce the build failure. AFAIU, the build error can only
> happen if for some reason the ib.h file got deleted
> accidentally. I did delete ib.h file and saw below error
> 
>   CC [M]  net/rds/rdma_transport.o
> net/rds/rdma_transport.c:38:16: error: ib.h: No such file or directory
> 
> Could it be the case for some reason the ib.h file got
> deleted or mangled while applying the $subject patch ?

Please post the patch again, I will try to sort it out.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] libbpf: support more map options
From: David Miller @ 2017-10-04  4:26 UTC (permalink / raw)
  To: kraigatgoog; +Cc: ast, daniel, brouer, chonggangli, netdev
In-Reply-To: <20171002164129.47986-1-kraigatgoog@gmail.com>

From: Craig Gallek <kraigatgoog@gmail.com>
Date: Mon,  2 Oct 2017 12:41:27 -0400

> From: Craig Gallek <kraig@google.com>
> 
> The functional change to this series is the ability to use flags when
> creating maps from object files loaded by libbpf.  In order to do this,
> the first patch updates the library to handle map definitions that
> differ in size from libbpf's struct bpf_map_def.
> 
> For object files with a larger map definition, libbpf will continue to load
> if the unknown fields are all zero, otherwise the map is rejected.  If the
> map definition in the object file is smaller than expected, libbpf will use
> zero as a default value in the missing fields.

Judging by the feedback I anticipate another spin of this series.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp: uniform the set up of sockets after successful connection
From: David Miller @ 2017-10-04  4:28 UTC (permalink / raw)
  To: weiwan; +Cc: netdev, ycheng, ncardwell, edumazet
In-Reply-To: <20171002170135.106183-1-tracywwnj@gmail.com>

From: Wei Wang <weiwan@google.com>
Date: Mon,  2 Oct 2017 10:01:35 -0700

> @@ -456,6 +456,19 @@ void tcp_init_sock(struct sock *sk)
>  }
>  EXPORT_SYMBOL(tcp_init_sock);
>  
> +void tcp_init_transfer(struct sock *sk, int bpf_op)
> +{
> +	struct inet_connection_sock *icsk = inet_csk(sk);
> +
> +	tcp_mtup_init(sk);
> +	icsk->icsk_af_ops->rebuild_header(sk);
> +	tcp_init_metrics(sk);
> +	tcp_call_bpf(sk, bpf_op);
> +	tcp_init_congestion_control(sk);
> +	tcp_init_buffer_space(sk);
> +}
> +EXPORT_SYMBOL(tcp_init_transfer);

This symbol export is unnecessary, and if it were it should
be EXPORT_SYMBOL_GPL().

^ permalink raw reply

* Re: [PATCH net-next v2 0/3] tools: add bpftool
From: David Miller @ 2017-10-04  4:33 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, daniel, alexei.starovoitov, oss-drivers
In-Reply-To: <20171002231130.12406-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon,  2 Oct 2017 16:11:27 -0700

> Hi!
> 
> This set adds bpftool to the tools/ directory.  The first 
> patch renames tools/net to tools/bpf, the second one adds 
> the new code, while the third adds simple documentation.
> 
> v2:
>  - report names, map ids, load time, uid;
>  - add docs/man pages;
>  - general cleanups & fixes.
> 
> Thanks to David Beckett for help with docs and testing.

I think, at the very least, the doc change requests deserve one more
respin.

Thanks.

^ permalink raw reply

* Re: [PATCH net] net: fib_rules: Fix fib_rules_ops->compare implementations to support exact match
From: David Ahern @ 2017-10-04  4:40 UTC (permalink / raw)
  To: Eyal Birger, David Miller
  Cc: shmulik, netdev@vger.kernel.org, mateusz.bajorski, tgraf,
	Shmulik Ladkani
In-Reply-To: <CAHsH6Gstsm7smQZ3A0qxfSsWvquT4HrLmMRzGae9L2Wcm7-Bbg@mail.gmail.com>

On 10/3/17 8:58 PM, Eyal Birger wrote:
> Hi David,
> 
> On Wed, Oct 4, 2017 at 12:54 AM, David Miller <davem@davemloft.net> wrote:
>> From: Shmulik Ladkani <shmulik@nsof.io>
>> Date: Sat, 30 Sep 2017 11:59:09 +0300
>>
>>> This leads to inconsistencies, depending on order of operations, e.g.:
>>
>> I don't see any inconsistency.  When you insert using NLM_F_EXCL the
>> insertion fails if any existing rule matches or overlaps in any way
>> with the keys in the new rule.
>>
>> Sorry I'm not going to apply this.
> 
> The inconsistency we saw is that 0.0.0.0/0 is treated differently compared to
> all other subnets - for which overlaps are not disallowed - e.g. this succeeds:
> 
> # ip ru add from 10.0.0.0/8 iif eth2 pref 33 table 33
> # ip ru add from 0.0.0.0/1 iif eth2 pref 33 table 33
> # ip ru add from 128.0.0.0/1 iif eth2 pref 33 table 33
> 
> Though being functionally equivalent to adding from=0.0.0.0/0.
> 
> So our understanding was that 'different subnet==different rule', similar to the
> route addition behavior with NLM_F_EXCL.
>

I agree with DaveM ... your "non-working" sequence has a specific entry
followed by a global, match all entry.

^ permalink raw reply

* Re: [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers
From: David Miller @ 2017-10-04  4:41 UTC (permalink / raw)
  To: fthain; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1506992619.git.fthain@telegraphics.com.au>

From: Finn Thain <fthain@telegraphics.com.au>
Date: Mon,  2 Oct 2017 21:07:17 -0400 (EDT)

> This patch series fixes some logging bugs and adds some missing message
> severity levels.
> 
> There are also cleanup patches for dead code and some Kconfig cruft.
> 
> Custom debug message logging is converted to netif_* calls to reduce
> code duplication.
> 
> All up, about 150 lines of code are eliminated.
> 
> My apologies for duplicated messages. I messed up the addressing.

Finn, I'm finding real bugs in this series and seriously if you cannot
test these changes in some way please leave this code alone.

For example, you're removing the "once_is_enough" logic from
mac89x0_probe().

But you can't do that.  The probe function can in fact be called
multiple times, from drivers/net/Space.c  It gets called in a loop
iterating over different 'unit' argument values.

Unless you're making stylistic changes where you can prove the object
code resulting is still the same, you really should not be playing
with fire by trying to remove "dead code" like this in legacy drivers
you cannot fully test.

Thank you.

^ permalink raw reply

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

From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Tue,  3 Oct 2017 11:10:53 +0530

> Add 0x6085 T6 device id.
> 
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH] fsl/fman: remove of_node
From: David Miller @ 2017-10-04  4:43 UTC (permalink / raw)
  To: madalin.bucur; +Cc: netdev, andrew, f.fainelli, linux-kernel
In-Reply-To: <AM5PR0402MB26914CBD37E3F28C17562258EC720@AM5PR0402MB2691.eurprd04.prod.outlook.com>

From: Madalin-cristian Bucur <madalin.bucur@nxp.com>
Date: Tue, 3 Oct 2017 08:49:31 +0000

> My patch removes the of_node that was set to a device that was not an
> of_device, preventing duplicated probing of both the real of_device
> and the "fake" one created through this assignment.
> 
> I understand that the DSA issue that triggered the initial change
> was related to DSA finding the network devices using 
> of_find_net_device_by_node(), something that will not work for the
> DPAA case where the netdevice does not have an of_node. I do not know
> enough about DSA to come up with a solution for this problem now.
> Andrew, Florian, can you please comment on this?

It sounds like you're knowingly breaking DSA.

^ permalink raw reply

* [PATCH net-next v3 0/3] bridge: neigh msg proxy and flood suppression support
From: Roopa Prabhu @ 2017-10-04  4:43 UTC (permalink / raw)
  To: davem; +Cc: netdev, nikolay, stephen, bridge

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This series implements arp and nd suppression in the bridge
driver for ethernet vpns. It implements rfc7432, section 10
https://tools.ietf.org/html/rfc7432#section-10
for ethernet VPN deployments. It is similar to the existing
BR_ARP_PROXY flag but has a few semantic differences to conform
to EVPN standard. In case of EVPN, it is mainly used to avoid flooding to
tunnel ports like vxlan. Unlike the existing flags it suppresses flood
of all neigh discovery packets (arp, nd) to tunnel ports.

v2 : rebase to latest + address some optimization feedback from Nikolay.
v3 : fix kbuild reported build errors with CONFIG_INET off

Roopa Prabhu (3):
  bridge: add new BR_NEIGH_SUPPRESS port flag to suppress arp and nd
    flood
  neigh arp suppress first
  bridge: suppress nd messages from going to BR_NEIGH_SUPPRESS ports

 include/linux/if_bridge.h    |   1 +
 include/uapi/linux/if_link.h |   1 +
 net/bridge/Makefile          |   2 +-
 net/bridge/br_arp_nd_proxy.c | 492 +++++++++++++++++++++++++++++++++++++++++++
 net/bridge/br_device.c       |  18 ++
 net/bridge/br_forward.c      |   3 +-
 net/bridge/br_if.c           |   5 +
 net/bridge/br_input.c        |  73 ++-----
 net/bridge/br_netlink.c      |  16 +-
 net/bridge/br_private.h      |   9 +
 net/bridge/br_sysfs_if.c     |   2 +
 11 files changed, 561 insertions(+), 61 deletions(-)
 create mode 100644 net/bridge/br_arp_nd_proxy.c

-- 
2.1.4

^ permalink raw reply

* [PATCH net-next v3 1/3] bridge: add new BR_NEIGH_SUPPRESS port flag to suppress arp and nd flood
From: Roopa Prabhu @ 2017-10-04  4:43 UTC (permalink / raw)
  To: davem; +Cc: netdev, nikolay, stephen, bridge
In-Reply-To: <1507092235-39876-1-git-send-email-roopa@cumulusnetworks.com>

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch adds a new bridge port flag BR_NEIGH_SUPPRESS to
suppress arp and nd flood on bridge ports. It implements
rfc7432, section 10.
https://tools.ietf.org/html/rfc7432#section-10
for ethernet VPN deployments. It is similar to the existing
BR_ARP_PROXY flag but has a few semantic differences to conform
to EVPN standard. In case of EVPN, it is mainly used to
avoid flooding to tunnel ports like vxlan. Unlike the existing
flags it suppresses flood of all neigh discovery packets
(arp, nd) to tunnel ports.

This patch adds netlink and sysfs support to set this bridge port
flag.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 include/linux/if_bridge.h    |  1 +
 include/uapi/linux/if_link.h |  1 +
 net/bridge/Makefile          |  2 +-
 net/bridge/br_arp_nd_proxy.c | 32 ++++++++++++++++++++++++++++++++
 net/bridge/br_forward.c      |  3 ++-
 net/bridge/br_if.c           |  5 +++++
 net/bridge/br_netlink.c      | 12 +++++++++++-
 net/bridge/br_private.h      |  2 ++
 net/bridge/br_sysfs_if.c     |  2 ++
 9 files changed, 57 insertions(+), 3 deletions(-)
 create mode 100644 net/bridge/br_arp_nd_proxy.c

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 3cd18ac..316ee11 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -49,6 +49,7 @@ struct br_ip_list {
 #define BR_MULTICAST_TO_UNICAST	BIT(12)
 #define BR_VLAN_TUNNEL		BIT(13)
 #define BR_BCAST_FLOOD		BIT(14)
+#define BR_NEIGH_SUPPRESS	BIT(15)
 
 #define BR_DEFAULT_AGEING_TIME	(300 * HZ)
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index ea87bd7..0d51f4f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -326,6 +326,7 @@ enum {
 	IFLA_BRPORT_VLAN_TUNNEL,
 	IFLA_BRPORT_BCAST_FLOOD,
 	IFLA_BRPORT_GROUP_FWD_MASK,
+	IFLA_BRPORT_NEIGH_SUPPRESS,
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index 40b1ede..4aee55f 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_BRIDGE) += bridge.o
 bridge-y	:= br.o br_device.o br_fdb.o br_forward.o br_if.o br_input.o \
 			br_ioctl.o br_stp.o br_stp_bpdu.o \
 			br_stp_if.o br_stp_timer.o br_netlink.o \
-			br_netlink_tunnel.o
+			br_netlink_tunnel.o br_arp_nd_proxy.o
 
 bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
 
diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
new file mode 100644
index 0000000..f889ad5
--- /dev/null
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -0,0 +1,32 @@
+/*
+ *  Handle bridge arp/nd proxy/suppress
+ *
+ *  Copyright (C) 2017 Cumulus Networks
+ *  Copyright (c) 2017 Roopa Prabhu <roopa@cumulusnetworks.com>
+ *
+ *  Authors:
+ *	Roopa Prabhu <roopa@cumulusnetworks.com>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include "br_private.h"
+
+void br_recalculate_neigh_suppress_enabled(struct net_bridge *br)
+{
+	struct net_bridge_port *p;
+	bool neigh_suppress = false;
+
+	list_for_each_entry(p, &br->port_list, list) {
+		if (p->flags & BR_NEIGH_SUPPRESS) {
+			neigh_suppress = true;
+			break;
+		}
+	}
+
+	br->neigh_suppress_enabled = neigh_suppress;
+}
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 48fb174..7a50dc5 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -204,7 +204,8 @@ void br_flood(struct net_bridge *br, struct sk_buff *skb,
 		/* Do not flood to ports that enable proxy ARP */
 		if (p->flags & BR_PROXYARP)
 			continue;
-		if ((p->flags & BR_PROXYARP_WIFI) &&
+		if ((p->flags & BR_PROXYARP_WIFI ||
+		     p->flags & BR_NEIGH_SUPPRESS) &&
 		    BR_INPUT_SKB_CB(skb)->proxyarp_replied)
 			continue;
 
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index f3aef22..8f615d4 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -310,6 +310,8 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
 		del_nbp(p);
 	}
 
+	br_recalculate_neigh_suppress_enabled(br);
+
 	br_fdb_delete_by_port(br, NULL, 0, 1);
 
 	cancel_delayed_work_sync(&br->gc_work);
@@ -653,4 +655,7 @@ void br_port_flags_change(struct net_bridge_port *p, unsigned long mask)
 
 	if (mask & BR_AUTO_MASK)
 		nbp_update_port_count(br);
+
+	if (mask & BR_NEIGH_SUPPRESS)
+		br_recalculate_neigh_suppress_enabled(br);
 }
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index dea88a2..d8c2706 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -138,6 +138,7 @@ static inline size_t br_port_info_size(void)
 		+ nla_total_size(1)	/* IFLA_BRPORT_PROXYARP */
 		+ nla_total_size(1)	/* IFLA_BRPORT_PROXYARP_WIFI */
 		+ nla_total_size(1)	/* IFLA_BRPORT_VLAN_TUNNEL */
+		+ nla_total_size(1)	/* IFLA_BRPORT_NEIGH_SUPPRESS */
 		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_ROOT_ID */
 		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_BRIDGE_ID */
 		+ nla_total_size(sizeof(u16))	/* IFLA_BRPORT_DESIGNATED_PORT */
@@ -210,7 +211,9 @@ static int br_port_fill_attrs(struct sk_buff *skb,
 	    nla_put_u8(skb, IFLA_BRPORT_CONFIG_PENDING, p->config_pending) ||
 	    nla_put_u8(skb, IFLA_BRPORT_VLAN_TUNNEL, !!(p->flags &
 							BR_VLAN_TUNNEL)) ||
-	    nla_put_u16(skb, IFLA_BRPORT_GROUP_FWD_MASK, p->group_fwd_mask))
+	    nla_put_u16(skb, IFLA_BRPORT_GROUP_FWD_MASK, p->group_fwd_mask) ||
+	    nla_put_u8(skb, IFLA_BRPORT_NEIGH_SUPPRESS, !!(p->flags &
+							BR_NEIGH_SUPPRESS)))
 		return -EMSGSIZE;
 
 	timerval = br_timer_value(&p->message_age_timer);
@@ -692,6 +695,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 {
 	unsigned long old_flags = p->flags;
 	bool br_vlan_tunnel_old = false;
+	int neigh_suppress_old = 0;
 	int err;
 
 	err = br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
@@ -785,6 +789,12 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 		p->group_fwd_mask = fwd_mask;
 	}
 
+	neigh_suppress_old = (p->flags & BR_NEIGH_SUPPRESS);
+	br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS,
+			 BR_NEIGH_SUPPRESS);
+	if (neigh_suppress_old != (p->flags & BR_NEIGH_SUPPRESS))
+		br_recalculate_neigh_suppress_enabled(p->br);
+
 	br_port_flags_change(p, old_flags ^ p->flags);
 	return 0;
 }
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 020c709..f47332e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -404,6 +404,7 @@ struct net_bridge {
 #ifdef CONFIG_NET_SWITCHDEV
 	int offload_fwd_mark;
 #endif
+	bool				neigh_suppress_enabled;
 };
 
 struct br_input_skb_cb {
@@ -1138,4 +1139,5 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb)
 }
 #endif /* CONFIG_NET_SWITCHDEV */
 
+void br_recalculate_neigh_suppress_enabled(struct net_bridge *br);
 #endif
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 9110d5e..0a1fa9c 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -191,6 +191,7 @@ BRPORT_ATTR_FLAG(proxyarp, BR_PROXYARP);
 BRPORT_ATTR_FLAG(proxyarp_wifi, BR_PROXYARP_WIFI);
 BRPORT_ATTR_FLAG(multicast_flood, BR_MCAST_FLOOD);
 BRPORT_ATTR_FLAG(broadcast_flood, BR_BCAST_FLOOD);
+BRPORT_ATTR_FLAG(neigh_suppress, BR_NEIGH_SUPPRESS);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -241,6 +242,7 @@ static const struct brport_attribute *brport_attrs[] = {
 	&brport_attr_multicast_flood,
 	&brport_attr_broadcast_flood,
 	&brport_attr_group_fwd_mask,
+	&brport_attr_neigh_suppress,
 	NULL
 };
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH net-next v3 2/3] bridge: suppress arp pkts on BR_NEIGH_SUPPRESS ports
From: Roopa Prabhu @ 2017-10-04  4:43 UTC (permalink / raw)
  To: davem; +Cc: netdev, nikolay, stephen, bridge
In-Reply-To: <1507092235-39876-1-git-send-email-roopa@cumulusnetworks.com>

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch avoids flooding and proxies arp packets
for BR_NEIGH_SUPPRESS ports.

Moves existing br_do_proxy_arp to br_do_proxy_suppress_arp
to support both proxy arp and neigh suppress.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 net/bridge/br_arp_nd_proxy.c | 186 +++++++++++++++++++++++++++++++++++++++++++
 net/bridge/br_device.c       |   9 +++
 net/bridge/br_input.c        |  63 ++-------------
 net/bridge/br_private.h      |   3 +
 4 files changed, 203 insertions(+), 58 deletions(-)

diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index f889ad5..6a31104 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -14,6 +14,13 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/neighbour.h>
+#include <net/arp.h>
+#include <linux/if_vlan.h>
+#include <linux/inetdevice.h>
+#include <net/addrconf.h>
 #include "br_private.h"
 
 void br_recalculate_neigh_suppress_enabled(struct net_bridge *br)
@@ -30,3 +37,182 @@ void br_recalculate_neigh_suppress_enabled(struct net_bridge *br)
 
 	br->neigh_suppress_enabled = neigh_suppress;
 }
+
+#if IS_ENABLED(CONFIG_INET)
+static void br_arp_send(struct net_bridge_port *p, int type, int ptype,
+			__be32 dest_ip, struct net_device *dev,
+			__be32 src_ip, const unsigned char *dest_hw,
+			const unsigned char *src_hw,
+			const unsigned char *target_hw,
+			__be16 vlan_proto, u16 vlan_tci)
+{
+	struct sk_buff *skb;
+
+	netdev_dbg(dev, "arp send dev %s dst %pI4 dst_hw %pM src %pI4 src_hw %pM\n",
+		   dev->name, &dest_ip, dest_hw, &src_ip, src_hw);
+
+	if (!vlan_tci) {
+		arp_send(type, ptype, dest_ip, dev, src_ip,
+			 dest_hw, src_hw, target_hw);
+		return;
+	}
+
+	skb = arp_create(type, ptype, dest_ip, dev, src_ip,
+			 dest_hw, src_hw, target_hw);
+	if (!skb)
+		return;
+
+	if (p) {
+		struct net_bridge_vlan_group *vg;
+		u16 pvid;
+
+		vg = nbp_vlan_group_rcu(p);
+		pvid = br_get_pvid(vg);
+		if (pvid == vlan_tci)
+			vlan_tci = 0;
+	}
+
+	if (vlan_tci) {
+		skb = vlan_insert_tag_set_proto(skb, vlan_proto,
+						vlan_tci);
+		if (!skb) {
+			net_err_ratelimited("%s: failed to insert VLAN tag\n",
+					    __func__);
+			return;
+		}
+	}
+
+	arp_xmit(skb);
+}
+
+static int br_chk_addr_ip(struct net_device *dev, void *data)
+{
+	__be32 ip = *(__be32 *)data;
+	struct in_device *in_dev;
+	__be32 addr = 0;
+
+	in_dev = __in_dev_get_rcu(dev);
+	if (in_dev)
+		addr = inet_confirm_addr(dev_net(dev), in_dev, 0, ip,
+					 RT_SCOPE_HOST);
+
+	if (addr == ip)
+		return 1;
+
+	return 0;
+}
+
+static bool br_is_local_ip(struct net_device *dev, __be32 ip)
+{
+	if (br_chk_addr_ip(dev, &ip))
+		return true;
+
+	/* check if ip is configured on upper dev */
+	if (netdev_walk_all_upper_dev_rcu(dev, br_chk_addr_ip, &ip))
+		return true;
+
+	return false;
+}
+
+void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
+			      u16 vid, struct net_bridge_port *p)
+{
+	struct net_device *dev = br->dev;
+	struct net_device *vlandev = dev;
+	struct neighbour *n;
+	struct arphdr *parp;
+	u8 *arpptr, *sha;
+	__be32 sip, tip;
+
+	BR_INPUT_SKB_CB(skb)->proxyarp_replied = false;
+
+	if ((dev->flags & IFF_NOARP) ||
+	    !pskb_may_pull(skb, arp_hdr_len(dev)))
+		return;
+
+	parp = arp_hdr(skb);
+
+	if (parp->ar_pro != htons(ETH_P_IP) ||
+	    parp->ar_hln != dev->addr_len ||
+	    parp->ar_pln != 4)
+		return;
+
+	arpptr = (u8 *)parp + sizeof(struct arphdr);
+	sha = arpptr;
+	arpptr += dev->addr_len;	/* sha */
+	memcpy(&sip, arpptr, sizeof(sip));
+	arpptr += sizeof(sip);
+	arpptr += dev->addr_len;	/* tha */
+	memcpy(&tip, arpptr, sizeof(tip));
+
+	if (ipv4_is_loopback(tip) ||
+	    ipv4_is_multicast(tip))
+		return;
+
+	if (br->neigh_suppress_enabled) {
+		if (p && (p->flags & BR_NEIGH_SUPPRESS))
+			return;
+		if (ipv4_is_zeronet(sip) || sip == tip) {
+			/* prevent flooding to neigh suppress ports */
+			BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+			return;
+		}
+	}
+
+	if (parp->ar_op != htons(ARPOP_REQUEST))
+		return;
+
+	if (vid != 0) {
+		vlandev = __vlan_find_dev_deep_rcu(br->dev, skb->vlan_proto,
+						   vid);
+		if (!vlandev)
+			return;
+	}
+
+	if (br->neigh_suppress_enabled && br_is_local_ip(vlandev, tip)) {
+		/* its our local ip, so don't proxy reply
+		 * and don't forward to neigh suppress ports
+		 */
+		BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+		return;
+	}
+
+	n = neigh_lookup(&arp_tbl, &tip, vlandev);
+	if (n) {
+		struct net_bridge_fdb_entry *f;
+
+		if (!(n->nud_state & NUD_VALID)) {
+			neigh_release(n);
+			return;
+		}
+
+		f = br_fdb_find_rcu(br, n->ha, vid);
+		if (f) {
+			bool replied = false;
+
+			if ((p && (p->flags & BR_PROXYARP)) ||
+			    (f->dst && ((f->dst->flags & BR_PROXYARP_WIFI) ||
+				       (f->dst->flags & BR_NEIGH_SUPPRESS)))) {
+				if (!vid)
+					br_arp_send(p, ARPOP_REPLY, ETH_P_ARP,
+						    sip, skb->dev, tip, sha,
+						    n->ha, sha, 0, 0);
+				else
+					br_arp_send(p, ARPOP_REPLY, ETH_P_ARP,
+						    sip, skb->dev, tip, sha,
+						    n->ha, sha, skb->vlan_proto,
+						    skb_vlan_tag_get(skb));
+				replied = true;
+			}
+
+			/* If we have replied or as long as we know the
+			 * mac, indicate to arp replied
+			 */
+			if (replied || br->neigh_suppress_enabled)
+				BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+		}
+
+		neigh_release(n);
+	}
+}
+#endif
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index f6b6a92..53d1456 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -39,6 +39,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct pcpu_sw_netstats *brstats = this_cpu_ptr(br->stats);
 	const struct nf_br_ops *nf_ops;
 	const unsigned char *dest;
+	struct ethhdr *eth;
 	u16 vid = 0;
 
 	rcu_read_lock();
@@ -57,11 +58,19 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	BR_INPUT_SKB_CB(skb)->brdev = dev;
 
 	skb_reset_mac_header(skb);
+	eth = eth_hdr(skb);
 	skb_pull(skb, ETH_HLEN);
 
 	if (!br_allowed_ingress(br, br_vlan_group_rcu(br), skb, &vid))
 		goto out;
 
+	if (IS_ENABLED(CONFIG_INET) &&
+	    (eth->h_proto == htons(ETH_P_ARP) ||
+	     eth->h_proto == htons(ETH_P_RARP)) &&
+	    br->neigh_suppress_enabled) {
+		br_do_proxy_suppress_arp(skb, br, vid, NULL);
+	}
+
 	dest = eth_hdr(skb)->h_dest;
 	if (is_broadcast_ether_addr(dest)) {
 		br_flood(br, skb, BR_PKT_BROADCAST, false, true);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 7cb6137..4b8d2ec 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -71,62 +71,6 @@ static int br_pass_frame_up(struct sk_buff *skb)
 		       br_netif_receive_skb);
 }
 
-static void br_do_proxy_arp(struct sk_buff *skb, struct net_bridge *br,
-			    u16 vid, struct net_bridge_port *p)
-{
-	struct net_device *dev = br->dev;
-	struct neighbour *n;
-	struct arphdr *parp;
-	u8 *arpptr, *sha;
-	__be32 sip, tip;
-
-	BR_INPUT_SKB_CB(skb)->proxyarp_replied = false;
-
-	if ((dev->flags & IFF_NOARP) ||
-	    !pskb_may_pull(skb, arp_hdr_len(dev)))
-		return;
-
-	parp = arp_hdr(skb);
-
-	if (parp->ar_pro != htons(ETH_P_IP) ||
-	    parp->ar_op != htons(ARPOP_REQUEST) ||
-	    parp->ar_hln != dev->addr_len ||
-	    parp->ar_pln != 4)
-		return;
-
-	arpptr = (u8 *)parp + sizeof(struct arphdr);
-	sha = arpptr;
-	arpptr += dev->addr_len;	/* sha */
-	memcpy(&sip, arpptr, sizeof(sip));
-	arpptr += sizeof(sip);
-	arpptr += dev->addr_len;	/* tha */
-	memcpy(&tip, arpptr, sizeof(tip));
-
-	if (ipv4_is_loopback(tip) ||
-	    ipv4_is_multicast(tip))
-		return;
-
-	n = neigh_lookup(&arp_tbl, &tip, dev);
-	if (n) {
-		struct net_bridge_fdb_entry *f;
-
-		if (!(n->nud_state & NUD_VALID)) {
-			neigh_release(n);
-			return;
-		}
-
-		f = br_fdb_find_rcu(br, n->ha, vid);
-		if (f && ((p->flags & BR_PROXYARP) ||
-			  (f->dst && (f->dst->flags & BR_PROXYARP_WIFI)))) {
-			arp_send(ARPOP_REPLY, ETH_P_ARP, sip, skb->dev, tip,
-				 sha, n->ha, sha);
-			BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
-		}
-
-		neigh_release(n);
-	}
-}
-
 /* note: already called with rcu_read_lock */
 int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
@@ -171,8 +115,11 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 
 	BR_INPUT_SKB_CB(skb)->brdev = br->dev;
 
-	if (IS_ENABLED(CONFIG_INET) && skb->protocol == htons(ETH_P_ARP))
-		br_do_proxy_arp(skb, br, vid, p);
+	if (IS_ENABLED(CONFIG_INET) &&
+	    (skb->protocol == htons(ETH_P_ARP) ||
+	     skb->protocol == htons(ETH_P_RARP))) {
+		br_do_proxy_suppress_arp(skb, br, vid, p);
+	}
 
 	switch (pkt_type) {
 	case BR_PKT_MULTICAST:
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f47332e..bb095dc 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1139,5 +1139,8 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb)
 }
 #endif /* CONFIG_NET_SWITCHDEV */
 
+/* br_arp_nd_proxy.c */
 void br_recalculate_neigh_suppress_enabled(struct net_bridge *br);
+void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
+			      u16 vid, struct net_bridge_port *p);
 #endif
-- 
2.1.4

^ permalink raw reply related

* [PATCH net-next v3 3/3] bridge: suppress nd pkts on BR_NEIGH_SUPPRESS ports
From: Roopa Prabhu @ 2017-10-04  4:43 UTC (permalink / raw)
  To: davem; +Cc: netdev, nikolay, stephen, bridge
In-Reply-To: <1507092235-39876-1-git-send-email-roopa@cumulusnetworks.com>

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch avoids flooding and proxies ndisc packets
for BR_NEIGH_SUPPRESS ports.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 net/bridge/br_arp_nd_proxy.c | 246 +++++++++++++++++++++++++++++++++++++++++++
 net/bridge/br_device.c       |  11 ++
 net/bridge/br_input.c        |  10 ++
 net/bridge/br_private.h      |   3 +
 4 files changed, 270 insertions(+)

diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index 6a31104..58cf174 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -216,3 +216,249 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
 	}
 }
 #endif
+
+#if IS_ENABLED(CONFIG_IPV6)
+struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb, struct nd_msg *msg)
+{
+	struct nd_msg *m;
+
+	m = skb_header_pointer(skb, skb_network_offset(skb) +
+			       sizeof(struct ipv6hdr), sizeof(*msg), msg);
+	if (!m)
+		return NULL;
+
+	if (m->icmph.icmp6_code != 0 ||
+	    (m->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION &&
+	     m->icmph.icmp6_type != NDISC_NEIGHBOUR_ADVERTISEMENT))
+		return NULL;
+
+	return m;
+}
+
+static void br_nd_send(struct net_bridge_port *p, struct sk_buff *request,
+		       struct neighbour *n, __be16 vlan_proto, u16 vlan_tci,
+		       struct nd_msg *ns)
+{
+	struct net_device *dev = request->dev;
+	struct sk_buff *reply;
+	struct nd_msg *na;
+	struct ipv6hdr *pip6;
+	u8 *daddr;
+	int na_olen = 8; /* opt hdr + ETH_ALEN for target */
+	int ns_olen;
+	int i, len;
+
+	if (!dev)
+		return;
+
+	len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
+		sizeof(*na) + na_olen + dev->needed_tailroom;
+
+	reply = alloc_skb(len, GFP_ATOMIC);
+	if (!reply)
+		return;
+
+	reply->protocol = htons(ETH_P_IPV6);
+	reply->dev = dev;
+	skb_reserve(reply, LL_RESERVED_SPACE(dev));
+	skb_push(reply, sizeof(struct ethhdr));
+	skb_set_mac_header(reply, 0);
+
+	daddr = eth_hdr(request)->h_source;
+
+	/* Do we need option processing ? */
+	ns_olen = request->len - (skb_network_offset(request) +
+				  sizeof(struct ipv6hdr)) - sizeof(*ns);
+	for (i = 0; i < ns_olen - 1; i += (ns->opt[i + 1] << 3)) {
+		if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
+			daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
+			break;
+		}
+	}
+
+	/* Ethernet header */
+	ether_addr_copy(eth_hdr(reply)->h_dest, daddr);
+	ether_addr_copy(eth_hdr(reply)->h_source, n->ha);
+	eth_hdr(reply)->h_proto = htons(ETH_P_IPV6);
+	reply->protocol = htons(ETH_P_IPV6);
+
+	skb_pull(reply, sizeof(struct ethhdr));
+	skb_set_network_header(reply, 0);
+	skb_put(reply, sizeof(struct ipv6hdr));
+
+	/* IPv6 header */
+	pip6 = ipv6_hdr(reply);
+	memset(pip6, 0, sizeof(struct ipv6hdr));
+	pip6->version = 6;
+	pip6->priority = ipv6_hdr(request)->priority;
+	pip6->nexthdr = IPPROTO_ICMPV6;
+	pip6->hop_limit = 255;
+	pip6->daddr = ipv6_hdr(request)->saddr;
+	pip6->saddr = *(struct in6_addr *)n->primary_key;
+
+	skb_pull(reply, sizeof(struct ipv6hdr));
+	skb_set_transport_header(reply, 0);
+
+	na = (struct nd_msg *)skb_put(reply, sizeof(*na) + na_olen);
+
+	/* Neighbor Advertisement */
+	memset(na, 0, sizeof(*na) + na_olen);
+	na->icmph.icmp6_type = NDISC_NEIGHBOUR_ADVERTISEMENT;
+	na->icmph.icmp6_router = 0; /* XXX: should be 1 ? */
+	na->icmph.icmp6_override = 1;
+	na->icmph.icmp6_solicited = 1;
+	na->target = ns->target;
+	ether_addr_copy(&na->opt[2], n->ha);
+	na->opt[0] = ND_OPT_TARGET_LL_ADDR;
+	na->opt[1] = na_olen >> 3;
+
+	na->icmph.icmp6_cksum = csum_ipv6_magic(&pip6->saddr,
+						&pip6->daddr,
+						sizeof(*na) + na_olen,
+						IPPROTO_ICMPV6,
+						csum_partial(na, sizeof(*na) + na_olen, 0));
+
+	pip6->payload_len = htons(sizeof(*na) + na_olen);
+
+	skb_push(reply, sizeof(struct ipv6hdr));
+	skb_push(reply, sizeof(struct ethhdr));
+
+	reply->ip_summed = CHECKSUM_UNNECESSARY;
+
+	if (p) {
+		struct net_bridge_vlan_group *vg;
+		u16 pvid;
+
+		vg = nbp_vlan_group_rcu(p);
+		pvid = br_get_pvid(vg);
+		if (pvid && pvid == vlan_tci)
+			vlan_tci = 0;
+	}
+
+	if (vlan_tci != 0) {
+		reply = vlan_insert_tag_set_proto(reply, vlan_proto, vlan_tci);
+		if (!reply) {
+			net_err_ratelimited("evpn: failed to insert VLAN tag\n");
+			return;
+		}
+	}
+
+	netdev_dbg(dev, "nd send dev %s dst %pI6 dst_hw %pM src %pI6 src_hw %pM\n",
+		   dev->name, &pip6->daddr, daddr, &pip6->saddr, n->ha);
+
+	dev_queue_xmit(reply);
+}
+
+static int br_chk_addr_ip6(struct net_device *dev, void *data)
+{
+	struct in6_addr *addr = (struct in6_addr *)data;
+
+	if (ipv6_chk_addr(dev_net(dev), addr, dev, 0))
+		return 1;
+
+	return 0;
+}
+
+static bool br_is_local_ip6(struct net_device *dev, struct in6_addr *addr)
+
+{
+	if (br_chk_addr_ip6(dev, addr))
+		return true;
+
+	/* check if ip is configured on upper dev */
+	if (netdev_walk_all_upper_dev_rcu(dev, br_chk_addr_ip6, addr))
+		return true;
+
+	return false;
+}
+
+void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
+		       u16 vid, struct net_bridge_port *p, struct nd_msg *msg)
+{
+	struct net_device *dev = br->dev;
+	struct net_device *vlandev = NULL;
+	struct in6_addr *saddr, *daddr;
+	struct ipv6hdr *iphdr;
+	struct inet6_dev *in6_dev;
+	struct neighbour *n;
+
+	BR_INPUT_SKB_CB(skb)->proxyarp_replied = false;
+
+	if (p && (p->flags & BR_NEIGH_SUPPRESS))
+		return;
+
+	if (msg->icmph.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT &&
+	    !msg->icmph.icmp6_solicited) {
+		/* prevent flooding to neigh suppress ports */
+		BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+		return;
+	}
+
+	if (msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
+		return;
+
+	in6_dev = __in6_dev_get(dev);
+	if (!in6_dev)
+		return;
+
+	iphdr = ipv6_hdr(skb);
+	saddr = &iphdr->saddr;
+	daddr = &iphdr->daddr;
+
+	if (ipv6_addr_any(saddr) || !ipv6_addr_cmp(saddr, daddr)) {
+		/* prevent flooding to neigh suppress ports */
+		BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+		return;
+	}
+
+	if (vid != 0) {
+		/* build neigh table lookup on the vlan device */
+		vlandev = __vlan_find_dev_deep_rcu(br->dev, skb->vlan_proto,
+						   vid);
+		if (!vlandev)
+			return;
+	} else {
+		vlandev = dev;
+	}
+
+	if (br_is_local_ip6(vlandev, &msg->target)) {
+		/* its our own ip, so don't proxy reply
+		 * and don't forward to arp suppress ports
+		 */
+		BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+		return;
+	}
+
+	n = neigh_lookup(ipv6_stub->nd_tbl, &msg->target, vlandev);
+	if (n) {
+		struct net_bridge_fdb_entry *f;
+
+		if (!(n->nud_state & NUD_VALID)) {
+			neigh_release(n);
+			return;
+		}
+
+		f = br_fdb_find_rcu(br, n->ha, vid);
+		if (f) {
+			bool replied = false;
+
+			if (f->dst && (f->dst->flags & BR_NEIGH_SUPPRESS)) {
+				if (vid != 0)
+					br_nd_send(p, skb, n, skb->vlan_proto,
+						   skb_vlan_tag_get(skb), msg);
+				else
+					br_nd_send(p, skb, n, 0, 0, msg);
+				replied = true;
+			}
+
+			/* If we have replied or as long as we know the
+			 * mac, indicate to NEIGH_SUPPRESS ports that we
+			 * have replied
+			 */
+			if (replied || br->neigh_suppress_enabled)
+				BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+		}
+		neigh_release(n);
+	}
+}
+#endif
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 53d1456..c85345a 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -69,6 +69,17 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	     eth->h_proto == htons(ETH_P_RARP)) &&
 	    br->neigh_suppress_enabled) {
 		br_do_proxy_suppress_arp(skb, br, vid, NULL);
+	} else if (IS_ENABLED(CONFIG_IPV6) &&
+		   skb->protocol == htons(ETH_P_IPV6) &&
+		   br->neigh_suppress_enabled &&
+		   pskb_may_pull(skb, sizeof(struct ipv6hdr) +
+				 sizeof(struct nd_msg)) &&
+		   ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
+			struct nd_msg *msg, _msg;
+
+			msg = br_is_nd_neigh_msg(skb, &_msg);
+			if (msg)
+				br_do_suppress_nd(skb, br, vid, NULL, msg);
 	}
 
 	dest = eth_hdr(skb)->h_dest;
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 4b8d2ec..013b65f 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -119,6 +119,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 	    (skb->protocol == htons(ETH_P_ARP) ||
 	     skb->protocol == htons(ETH_P_RARP))) {
 		br_do_proxy_suppress_arp(skb, br, vid, p);
+	} else if (IS_ENABLED(CONFIG_IPV6) && br->neigh_suppress_enabled &&
+		   skb->protocol == htons(ETH_P_IPV6) &&
+		   pskb_may_pull(skb, sizeof(struct ipv6hdr) +
+				 sizeof(struct nd_msg)) &&
+		   ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
+			struct nd_msg *msg, _msg;
+
+			msg = br_is_nd_neigh_msg(skb, &_msg);
+			if (msg)
+				br_do_suppress_nd(skb, br, vid, p, msg);
 	}
 
 	switch (pkt_type) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index bb095dc..f6936e9 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1143,4 +1143,7 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb)
 void br_recalculate_neigh_suppress_enabled(struct net_bridge *br);
 void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
 			      u16 vid, struct net_bridge_port *p);
+void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
+		       u16 vid, struct net_bridge_port *p, struct nd_msg *msg);
+struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb, struct nd_msg *m);
 #endif
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH 2/5] VSOCK: export __vsock_in_bound/connected_table()
From: David Miller @ 2017-10-04  4:46 UTC (permalink / raw)
  To: stefanha; +Cc: netdev, jhansen, decui
In-Reply-To: <20171003153943.23159-3-stefanha@redhat.com>

From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue,  3 Oct 2017 11:39:40 -0400

> @@ -250,15 +250,17 @@ static struct sock *__vsock_find_connected_socket(struct sockaddr_vm *src,
>  	return NULL;
>  }
>  
> -static bool __vsock_in_bound_table(struct vsock_sock *vsk)
> +bool __vsock_in_bound_table(struct vsock_sock *vsk)
>  {
>  	return !list_empty(&vsk->bound_table);
>  }
> +EXPORT_SYMBOL_GPL(__vsock_in_bound_table);
>  
> -static bool __vsock_in_connected_table(struct vsock_sock *vsk)
> +bool __vsock_in_connected_table(struct vsock_sock *vsk)
>  {
>  	return !list_empty(&vsk->connected_table);
>  }
> +EXPORT_SYMBOL_GPL(__vsock_in_connected_table);

Maybe you can just make these inline helpers in af_vsock.h?

^ permalink raw reply

* Re: [PATCH 4/5] VSOCK: add sock_diag interface
From: David Miller @ 2017-10-04  4:46 UTC (permalink / raw)
  To: stefanha; +Cc: netdev, jhansen, decui
In-Reply-To: <20171003153943.23159-5-stefanha@redhat.com>

From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue,  3 Oct 2017 11:39:42 -0400

> +static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
> +			u32 portid, u32 seq, u32 flags)
> +{
> +	struct nlmsghdr *nlh;
> +	struct vsock_diag_msg *rep;
> +	struct vsock_sock *vsk = vsock_sk(sk);

Please order local variables from longest to shortest line.

> +static int vsock_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> +	struct vsock_diag_req *req;
> +	unsigned int table;
> +	unsigned int bucket;
> +	unsigned int last_i;
> +	unsigned int i;
> +	struct vsock_sock *vsk;
> +	struct net *net;

Likewise.

^ permalink raw reply

* Re: [PATCH net-next v2 1/3] bridge: add new BR_NEIGH_SUPPRESS port flag to suppress arp and nd flood
From: Roopa Prabhu @ 2017-10-04  4:47 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov,
	bridge
In-Reply-To: <20171003112948.44d9cc56@xeon-e3>

On Tue, Oct 3, 2017 at 11:29 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue,  3 Oct 2017 11:21:14 -0700
> Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
>> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
>> index 48fb174..7a50dc5 100644
>> --- a/net/bridge/br_forward.c
>> +++ b/net/bridge/br_forward.c
>> @@ -204,7 +204,8 @@ void br_flood(struct net_bridge *br, struct sk_buff *skb,
>>               /* Do not flood to ports that enable proxy ARP */
>>               if (p->flags & BR_PROXYARP)
>>                       continue;
>> -             if ((p->flags & BR_PROXYARP_WIFI) &&
>> +             if ((p->flags & BR_PROXYARP_WIFI ||
>> +                  p->flags & BR_NEIGH_SUPPRESS) &&
>>                   BR_INPUT_SKB_CB(skb)->proxyarp_replied)
>>                       continue;
>
> Don;t you need additional paren here to avoid warnings.
> Or do one mask:
>                 if ((p->flags & (BR_PROXYARP_WIFI | BR_NEIGH_SUPPRESS)) &&
>                     BR_INPUT_SKB_CB(skb)->proxyarp_replied)
>                         continue;

Missed seeing this, i just posted v3. I did not get a warning. I can
certainly simplify the mask and post.

^ permalink raw reply

* Re: [PATCH 5/5] VSOCK: add tools/vsock/vsock_diag_test
From: David Miller @ 2017-10-04  4:48 UTC (permalink / raw)
  To: stefanha; +Cc: netdev, jhansen, decui
In-Reply-To: <20171003153943.23159-6-stefanha@redhat.com>

From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue,  3 Oct 2017 11:39:43 -0400

>  MAINTAINERS                   |   1 +
>  tools/vsock/Makefile          |   9 +
>  tools/vsock/control.h         |  13 +
>  tools/vsock/timeout.h         |  14 +
>  tools/vsock/control.c         | 219 ++++++++++++++
>  tools/vsock/timeout.c         |  64 ++++
>  tools/vsock/vsock_diag_test.c | 681 ++++++++++++++++++++++++++++++++++++++++++
>  tools/vsock/.gitignore        |   2 +

Please don't create you own "special" directory for tests.

Tests belong under tools/testing/selftests/

If you put your tests in the proper place, and structure them properly (especially
the Makefile rules), they will automatically be run by various automated build
and test frameworks.

^ permalink raw reply

* Re: [PATCH net] net: br: Fix igmp snooping offload with CONFIG_BRIDGE_VLAN_FILTERING
From: Toshiaki Makita @ 2017-10-04  4:52 UTC (permalink / raw)
  To: Ido Schimmel, Vivien Didelot, Andrew Lunn
  Cc: Toshiaki Makita, David Miller, netdev
In-Reply-To: <20171003164211.GA5177@shredder.mtl.com>

On 2017/10/04 1:42, Ido Schimmel wrote:
> On Tue, Oct 03, 2017 at 12:25:08PM -0400, Vivien Didelot wrote:
>> Andrew Lunn <andrew@lunn.ch> writes:
>>
>>>> The vlan will be effective only when vlan_filtering is enabled.
>>>> When vlan_filtering is disabled, vlan information is still kept in the
>>>> bridge and gets effective later when vlan_filtering becomes enable.
>>>
>>> O.K, so things are starting to get clearer.
>>>
>>> So when vlan filtering is disabled, the hardware should just ignore
>>> the requests to add the vlan to the hardware?
>>>
>>> When vlan_filtering is enabled, are all the vlans in the software
>>> bridge again offloaded? Or do we need to remember all the vlans which
>>> we ignored while vlan filtering was disabled? The average switch has
>>> nowhere to store these disabled vlans. It can only store active vlans.

Seems that __br_vlan_filter_toggle() only propagates
SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING.
IMHO vlan-related objects (vlan, fdb, and mdb) should be remembered if
vlan_filtering can be enabled later. But this sounds redundant as the
same information is maintained in the bridge so I'm not sure this is the
best way.

>>
>> When vlan_filtering is enabled on the bridge, the bridge code does
>> propagates the default_pvid again if I recall correctly.

I couldn't find it in the source...

>>
>> In my opinion the hardware mustn't ignore the VLAN requests, because we
>> seem to agree that vlan_filtering disabled means that the target ports
>> should not care yet about 802.1Q. So having some unused hardware VLAN
>> entries and some ports with disabled 802.1Q mode must work together.

Probably I don't fully understand you, but I think hardware can ignore
VLAN requests while vlan_filtering is disabled, as long as they are
properly populated to hardware on enabling vlan_filtering.

>>
>> That being said we still have the wrong hardware FDB populated when
>> CONFIG_BRIDGE_VLAN_FILTERING is enabled but not vlan_filtering...
> 
> The driver can make sure it's able to handle the configured
> `vlan_filtering` state during port enslavement to the bridge and also
> forbid it from being toggled once it's enslaved.

That is a simple solution.
One concern is backward compatibility. I wonder if we can prohibit
toggling for some driver which currently allows it.

-- 
Toshiaki Makita

^ permalink raw reply

* [PATCH net-next 0/7] net: Plumb extack error reporting to enslavements
From: David Ahern @ 2017-10-04  4:58 UTC (permalink / raw)
  To: netdev; +Cc: j.vosburgh, vfalico, andy, jiri, idosch, davem, bridge,
	David Ahern

Another round of extending extack error reporting, this time for
enslavements through ndo_add_slave and notifiers.

David Ahern (7):
  net: Add extack to netdev_notifier_info
  net: Add extack to ndo_add_slave
  net: Add extack to upper device linking
  net: vrf: Add extack messages for enslave errors
  net: bonding: Add extack messages for some enslave failures
  net: bridge: Pass extack to down to netdev_master_upper_dev_link
  mlxsw: spectrum: Add extack messages for enslave failures

 drivers/net/bonding/bond_main.c                    | 24 ++++--
 drivers/net/bonding/bond_options.c                 |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 46 ++++++++---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c |  2 +-
 drivers/net/hyperv/netvsc_drv.c                    |  2 +-
 drivers/net/ipvlan/ipvlan_main.c                   |  2 +-
 drivers/net/macsec.c                               |  2 +-
 drivers/net/macvlan.c                              |  7 +-
 drivers/net/macvtap.c                              |  2 +-
 drivers/net/team/team.c                            |  5 +-
 drivers/net/usb/qmi_wwan.c                         |  2 +-
 drivers/net/vrf.c                                  | 23 ++++--
 include/linux/if_macvlan.h                         |  3 +-
 include/linux/netdevice.h                          | 19 ++++-
 include/net/bonding.h                              |  3 +-
 net/8021q/vlan.c                                   |  6 +-
 net/8021q/vlan.h                                   |  2 +-
 net/8021q/vlan_netlink.c                           |  2 +-
 net/batman-adv/hard-interface.c                    |  2 +-
 net/batman-adv/soft-interface.c                    |  3 +-
 net/bridge/br_device.c                             |  5 +-
 net/bridge/br_if.c                                 | 15 +++-
 net/bridge/br_ioctl.c                              |  2 +-
 net/bridge/br_private.h                            |  3 +-
 net/core/dev.c                                     | 96 +++++++++++++---------
 net/core/rtnetlink.c                               | 10 ++-
 net/openvswitch/vport-netdev.c                     |  3 +-
 27 files changed, 196 insertions(+), 97 deletions(-)

-- 
2.1.4

^ 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