Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2 rfc 0/8] IGMP snooping for local traffic
From: Matthias May @ 2017-09-06 14:46 UTC (permalink / raw)
  To: Andrew Lunn, netdev, Florian Fainelli, Vivien Didelot,
	Woojung.Huh, jbe, sean.wang, john
In-Reply-To: <20170906004703.GB27385@lunn.ch>

On 06/09/17 02:47, Andrew Lunn wrote:
>> The third and last issue will be explained in a followup email.
> 
> Hi DSA hackers
> 
> So there is the third issue. It affects just DSA, but it possible
> affects all DSA drivers.
> 
> This patchset broken broadcast with the Marvell drivers. It could
> break broadcast on others drivers as well.
> 
> What i found is that the Marvell chips don't flood broadcast frames
> between bridged ports. What appears to happen is there is a fdb miss,
> so it gets forwarded to the CPU port for the host to deal with. The
> software bridge when floods it out all ports of the bridge.
> 
> But the set offload_fwd_mark patch changes this. The software bridge
> now assumes the hardware has already flooded broadcast out all ports
> of the switch as needed. So it does not do any flooding itself. As a
> result, on Marvell devices, broadcast packets don't get flooded at
> all.
> 
> The issue can be fixed. I just need to add an mdb entry for the
> broadcast address to each port of the bridge in the switch, and the
> CPU port.  But i don't know at what level to do this.
> 
> Should this be done at the DSA level, or at the driver level?  Do any
> chips do broadcast flooding in hardware already? Hence they currently
> see broadcast duplication? If i add a broadcast mdb at the DSA level,
> and the chip is already hard wired to flooding broadcast, is it going
> to double flood?
> 
> 	Andrew
> 

Hi Andrew
We are using the 88E6321.
In our setup we are using openvswitch and not a bridge, however the problem you describe seems to be the same.

We had to configure the switch to flood unknown multicast (Egress Floods = 0x3, bits 3:2, offset 0x4 in port control)
and
unset FloodBC (FloodBC = 0x0, bit 12, offset 0x5 in global 2) which defines if a broadcast should be considered as
multicast for the above config.

Regarding the looping problem:
Since OVS isn't aware of the fdb of the switch, we configure the ports representing the switch ports (in ovs) as
"protected" which prevents looping them back between (even when flooding) see [1].
I'm not sure if the bridge has a similar feature.

BR
Matthias

[1]http://openvswitch.org/support/dist-docs/ovs-vswitchd.conf.db.5.txt ctrl-f: "protected: boolean"

^ permalink raw reply

* Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
From: Mason @ 2017-09-06 14:55 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Marc Gonzalez, David Daney, netdev, Geert Uytterhoeven,
	David Miller, Andrew Lunn, Mans Rullgard, Thibaud Cornic
In-Reply-To: <7b1c1dc9-b6e3-a1bd-2e36-474946741a79@gmail.com>

On 31/08/2017 21:18, Florian Fainelli wrote:

> On 08/31/2017 12:09 PM, Mason wrote:
> 
>> 1) nb8800_link_reconfigure() calls phy_print_status()
>> which prints the "Link down" and "Link up" messages
>> to the console. With the patch reverted, nothing is
>> printed when the link goes down, and the result is
>> random when the link comes up. Sometimes, we get
>> down + up, sometimes just up.
> 
> Nothing printed when you bring down the network interface as a result of
> not signaling the link down, there is a small nuance here.

Let me first focus on the "Link down" message.

Do you agree that such a message should be printed when the
link goes down, not when the link comes up?

Perhaps the issue is that the 2 following cases need to be
handled differently:
A) operator sets link down on the command-line
B) asynchronous event makes link go down (peer is dead, cable is cut, etc)

In B) the PHY state machine keeps on running, and eventually
calls adjust_link()

In A) the driver calls phy_stop() and phy_disconnect() and
therefore adjust_link() will not be called?

Regards.

^ permalink raw reply

* [PATCH 2/2] net/9p: Switch to wait_event_killable()
From: Tuomas Tynkkynen @ 2017-09-06 14:59 UTC (permalink / raw)
  To: Al Viro
  Cc: v9fs-developer, Eric Van Hensbergen, Ron Minnich,
	Latchesar Ionkov, David S. Miller, linux-kernel, netdev,
	linux-fsdevel, Tuomas Tynkkynen, stable
In-Reply-To: <20170906145908.8082-1-tuomas@tuxera.com>

Because userspace gets Very Unhappy when calls like stat() and execve()
return -EINTR on 9p filesystem mounts. For instance, when bash is
looking in PATH for things to execute and some SIGCHLD interrupts
stat(), bash can throw a spurious 'command not found' since it doesn't
retry the stat().

In practice, hitting the problem is rare and needs a really
slow/bogged down 9p server.

Cc: stable@vger.kernel.org
Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
---
 net/9p/client.c       |  3 +--
 net/9p/trans_virtio.c | 13 ++++++-------
 net/9p/trans_xen.c    |  4 ++--
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index 4674235b0d9b..1beb131dd3e1 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -773,8 +773,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
 	}
 again:
 	/* Wait for the response */
-	err = wait_event_interruptible(*req->wq,
-				       req->status >= REQ_STATUS_RCVD);
+	err = wait_event_killable(*req->wq, req->status >= REQ_STATUS_RCVD);
 
 	/*
 	 * Make sure our req is coherent with regard to updates in other
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index f24b25c25106..f3a4efcf1456 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -286,8 +286,8 @@ p9_virtio_request(struct p9_client *client, struct p9_req_t *req)
 		if (err == -ENOSPC) {
 			chan->ring_bufs_avail = 0;
 			spin_unlock_irqrestore(&chan->lock, flags);
-			err = wait_event_interruptible(*chan->vc_wq,
-							chan->ring_bufs_avail);
+			err = wait_event_killable(*chan->vc_wq,
+						  chan->ring_bufs_avail);
 			if (err  == -ERESTARTSYS)
 				return err;
 
@@ -327,7 +327,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
 		 * Other zc request to finish here
 		 */
 		if (atomic_read(&vp_pinned) >= chan->p9_max_pages) {
-			err = wait_event_interruptible(vp_wq,
+			err = wait_event_killable(vp_wq,
 			      (atomic_read(&vp_pinned) < chan->p9_max_pages));
 			if (err == -ERESTARTSYS)
 				return err;
@@ -471,8 +471,8 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
 		if (err == -ENOSPC) {
 			chan->ring_bufs_avail = 0;
 			spin_unlock_irqrestore(&chan->lock, flags);
-			err = wait_event_interruptible(*chan->vc_wq,
-						       chan->ring_bufs_avail);
+			err = wait_event_killable(*chan->vc_wq,
+						  chan->ring_bufs_avail);
 			if (err  == -ERESTARTSYS)
 				goto err_out;
 
@@ -489,8 +489,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
 	virtqueue_kick(chan->vq);
 	spin_unlock_irqrestore(&chan->lock, flags);
 	p9_debug(P9_DEBUG_TRANS, "virtio request kicked\n");
-	err = wait_event_interruptible(*req->wq,
-				       req->status >= REQ_STATUS_RCVD);
+	err = wait_event_killable(*req->wq, req->status >= REQ_STATUS_RCVD);
 	/*
 	 * Non kernel buffers are pinned, unpin them
 	 */
diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 6ad3e043c617..325c56043007 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -156,8 +156,8 @@ static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 	ring = &priv->rings[num];
 
 again:
-	while (wait_event_interruptible(ring->wq,
-					p9_xen_write_todo(ring, size)) != 0)
+	while (wait_event_killable(ring->wq,
+				   p9_xen_write_todo(ring, size)) != 0)
 		;
 
 	spin_lock_irqsave(&ring->lock, flags);
-- 
2.13.0

^ permalink raw reply related

* Re: hung task in mac80211
From: Matteo Croce @ 2017-09-06 15:04 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1504702728.13457.17.camel@sipsolutions.net>

On Wed, Sep 6, 2017 at 2:58 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2017-09-06 at 13:57 +0200, Matteo Croce wrote:
>
>> I have an hung task on vanilla 4.13 kernel which I haven't on 4.12.
>> The problem is present both on my AP and on my notebook,
>> so it seems it affects AP and STA mode as well.
>> The generated messages are:
>>
>> INFO: task kworker/u16:6:120 blocked for more than 120 seconds.
>>       Not tainted 4.13.0 #57
>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
>> message.
>> kworker/u16:6   D    0   120      2 0x00000000
>> Workqueue: phy0 ieee80211_ba_session_work [mac80211]
>> Call Trace:
>>  ? __schedule+0x174/0x5b0
>>  ? schedule+0x31/0x80
>>  ? schedule_preempt_disabled+0x9/0x10
>>  ? __mutex_lock.isra.2+0x163/0x480
>>  ? select_task_rq_fair+0xb9f/0xc60
>>  ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>>  ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>
> Yeah - obviously as Stefano found, both take &sta->ampdu_mlme.mtx.
>
> Can you try this?
>
> diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
> index 2b36eff5d97e..d8d32776031e 100644
> --- a/net/mac80211/agg-rx.c
> +++ b/net/mac80211/agg-rx.c
> @@ -245,10 +245,10 @@ static void ieee80211_send_addba_resp(struct ieee80211_sub_if_data *sdata, u8 *d
>         ieee80211_tx_skb(sdata, skb);
>  }
>
> -void __ieee80211_start_rx_ba_session(struct sta_info *sta,
> -                                    u8 dialog_token, u16 timeout,
> -                                    u16 start_seq_num, u16 ba_policy, u16 tid,
> -                                    u16 buf_size, bool tx, bool auto_seq)
> +void ___ieee80211_start_rx_ba_session(struct sta_info *sta,
> +                                     u8 dialog_token, u16 timeout,
> +                                     u16 start_seq_num, u16 ba_policy, u16 tid,
> +                                     u16 buf_size, bool tx, bool auto_seq)
>  {
>         struct ieee80211_local *local = sta->sdata->local;
>         struct tid_ampdu_rx *tid_agg_rx;
> @@ -267,7 +267,7 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>                 ht_dbg(sta->sdata,
>                        "STA %pM requests BA session on unsupported tid %d\n",
>                        sta->sta.addr, tid);
> -               goto end_no_lock;
> +               goto end;
>         }
>
>         if (!sta->sta.ht_cap.ht_supported) {
> @@ -275,14 +275,14 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>                        "STA %pM erroneously requests BA session on tid %d w/o QoS\n",
>                        sta->sta.addr, tid);
>                 /* send a response anyway, it's an error case if we get here */
> -               goto end_no_lock;
> +               goto end;
>         }
>
>         if (test_sta_flag(sta, WLAN_STA_BLOCK_BA)) {
>                 ht_dbg(sta->sdata,
>                        "Suspend in progress - Denying ADDBA request (%pM tid %d)\n",
>                        sta->sta.addr, tid);
> -               goto end_no_lock;
> +               goto end;
>         }
>
>         /* sanity check for incoming parameters:
> @@ -296,7 +296,7 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>                 ht_dbg_ratelimited(sta->sdata,
>                                    "AddBA Req with bad params from %pM on tid %u. policy %d, buffer size %d\n",
>                                    sta->sta.addr, tid, ba_policy, buf_size);
> -               goto end_no_lock;
> +               goto end;
>         }
>         /* determine default buffer size */
>         if (buf_size == 0)
> @@ -311,7 +311,6 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>                buf_size, sta->sta.addr);
>
>         /* examine state machine */
> -       mutex_lock(&sta->ampdu_mlme.mtx);
>
>         if (test_bit(tid, sta->ampdu_mlme.agg_session_valid)) {
>                 if (sta->ampdu_mlme.tid_rx_token[tid] == dialog_token) {
> @@ -415,15 +414,25 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>                 __clear_bit(tid, sta->ampdu_mlme.unexpected_agg);
>                 sta->ampdu_mlme.tid_rx_token[tid] = dialog_token;
>         }
> -       mutex_unlock(&sta->ampdu_mlme.mtx);
>
> -end_no_lock:
>         if (tx)
>                 ieee80211_send_addba_resp(sta->sdata, sta->sta.addr, tid,
>                                           dialog_token, status, 1, buf_size,
>                                           timeout);
>  }
>
> +void __ieee80211_start_rx_ba_session(struct sta_info *sta,
> +                                    u8 dialog_token, u16 timeout,
> +                                    u16 start_seq_num, u16 ba_policy, u16 tid,
> +                                    u16 buf_size, bool tx, bool auto_seq)
> +{
> +       mutex_lock(&sta->ampdu_mlme.mtx);
> +       ___ieee80211_start_rx_ba_session(sta, dialog_token, timeout,
> +                                        start_seq_num, ba_policy, tid,
> +                                        buf_size, tx, auto_seq);
> +       mutex_unlock(&sta->ampdu_mlme.mtx);
> +}
> +
>  void ieee80211_process_addba_request(struct ieee80211_local *local,
>                                      struct sta_info *sta,
>                                      struct ieee80211_mgmt *mgmt,
> diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
> index c92df492e898..198b2d3e56fd 100644
> --- a/net/mac80211/ht.c
> +++ b/net/mac80211/ht.c
> @@ -333,9 +333,9 @@ void ieee80211_ba_session_work(struct work_struct *work)
>
>                 if (test_and_clear_bit(tid,
>                                        sta->ampdu_mlme.tid_rx_manage_offl))
> -                       __ieee80211_start_rx_ba_session(sta, 0, 0, 0, 1, tid,
> -                                                       IEEE80211_MAX_AMPDU_BUF,
> -                                                       false, true);
> +                       ___ieee80211_start_rx_ba_session(sta, 0, 0, 0, 1, tid,
> +                                                        IEEE80211_MAX_AMPDU_BUF,
> +                                                        false, true);
>
>                 if (test_and_clear_bit(tid + IEEE80211_NUM_TIDS,
>                                        sta->ampdu_mlme.tid_rx_manage_offl))
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 2197c62a0a6e..9675814f64db 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1760,6 +1760,10 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>                                      u8 dialog_token, u16 timeout,
>                                      u16 start_seq_num, u16 ba_policy, u16 tid,
>                                      u16 buf_size, bool tx, bool auto_seq);
> +void ___ieee80211_start_rx_ba_session(struct sta_info *sta,
> +                                     u8 dialog_token, u16 timeout,
> +                                     u16 start_seq_num, u16 ba_policy, u16 tid,
> +                                     u16 buf_size, bool tx, bool auto_seq);
>  void ieee80211_sta_tear_down_BA_sessions(struct sta_info *sta,
>                                          enum ieee80211_agg_stop_reason reason);
>  void ieee80211_process_delba(struct ieee80211_sub_if_data *sdata,
>
> johannes

I confirm that this patch fixes the hang too.
I'm curious to see if there are noticeable performance differences
between the two solutions.

Cheers,
-- 
Matteo Croce
per aspera ad upstream

^ permalink raw reply

* Re: [PATCH v2 rfc 8/8] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
From: Andrew Lunn @ 2017-09-06 15:08 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, jiri, nikolay, Florian Fainelli
In-Reply-To: <87d17351gn.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

> > Use the MAC address of the master interface as the parent ID. This is
> > the same for all switches in a cluster, and should be unique if there
> > are multiple clusters.
> 
> That is not correct. Support for multiple CPU ports is coming and in
> this case, you can have two CPU host interfaces wired to two switch
> ports of the same tree. So two different master MAC addresses.

Yes, you are correct. I will change this.

     Andrew

^ permalink raw reply

* [PATCH 0/2] 9p: Fixes for hard-to-hit bugs
From: Tuomas Tynkkynen @ 2017-09-06 14:59 UTC (permalink / raw)
  To: Al Viro
  Cc: v9fs-developer, Eric Van Hensbergen, Ron Minnich,
	Latchesar Ionkov, David S. Miller, linux-kernel, netdev,
	linux-fsdevel, Tuomas Tynkkynen

These two patches fix two hard-to-hit (but really annoying) bugs in 9p.
The first one was posted earlier in February (with one R-b), the second
is a new one.

Both of these have had soaking in NixOS distribution kernels for
a couple of months with no ill effects.

Tuomas Tynkkynen (2):
  fs/9p: Compare qid.path in v9fs_test_inode
  net/9p: Switch to wait_event_killable()

 fs/9p/vfs_inode.c      |  3 +++
 fs/9p/vfs_inode_dotl.c |  3 +++
 net/9p/client.c        |  3 +--
 net/9p/trans_virtio.c  | 13 ++++++-------
 net/9p/trans_xen.c     |  4 ++--
 5 files changed, 15 insertions(+), 11 deletions(-)

-- 
2.13.0

^ permalink raw reply

* [PATCH 1/2] fs/9p: Compare qid.path in v9fs_test_inode
From: Tuomas Tynkkynen @ 2017-09-06 14:59 UTC (permalink / raw)
  To: Al Viro
  Cc: v9fs-developer, Eric Van Hensbergen, Ron Minnich,
	Latchesar Ionkov, David S. Miller, linux-kernel, netdev,
	linux-fsdevel, Tuomas Tynkkynen, stable
In-Reply-To: <20170906145908.8082-1-tuomas@tuxera.com>

Commit fd2421f54423 ("fs/9p: When doing inode lookup compare qid details
and inode mode bits.") transformed v9fs_qid_iget() to use iget5_locked()
instead of iget_locked(). However, the test() callback is not checking
fid.path at all, which means that a lookup in the inode cache can now
accidentally locate a completely wrong inode from the same inode hash
bucket if the other fields (qid.type and qid.version) match.

Fixes: fd2421f54423 ("fs/9p: When doing inode lookup compare qid details and inode mode bits.")
Cc: stable@vger.kernel.org
Reviewed-by: Latchesar Ionkov <lucho@ionkov.net>
Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
---
 fs/9p/vfs_inode.c      | 3 +++
 fs/9p/vfs_inode_dotl.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 2a5de610dd8f..bdabb2765d1b 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -483,6 +483,9 @@ static int v9fs_test_inode(struct inode *inode, void *data)
 
 	if (v9inode->qid.type != st->qid.type)
 		return 0;
+
+	if (v9inode->qid.path != st->qid.path)
+		return 0;
 	return 1;
 }
 
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 70f9887c59a9..7f6ae21a27b3 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -87,6 +87,9 @@ static int v9fs_test_inode_dotl(struct inode *inode, void *data)
 
 	if (v9inode->qid.type != st->qid.type)
 		return 0;
+
+	if (v9inode->qid.path != st->qid.path)
+		return 0;
 	return 1;
 }
 
-- 
2.13.0

^ permalink raw reply related

* Re: hung task in mac80211
From: Johannes Berg @ 2017-09-06 15:11 UTC (permalink / raw)
  To: Matteo Croce; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <CAGnkfhyBMKT-Tf10NxjN40Znuj7OK2pSM3qLUidPSkQqh8gjwQ@mail.gmail.com>

On Wed, 2017-09-06 at 17:04 +0200, Matteo Croce wrote:
> 
> I confirm that this patch fixes the hang too.

Cool, I'll go apply it.

> I'm curious to see if there are noticeable performance differences
> between the two solutions.

Nope, you hit this code path essentially once.

johannes

^ permalink raw reply

* Re: [PATCH v2 rfc 0/8] IGMP snooping for local traffic
From: Andrew Lunn @ 2017-09-06 15:16 UTC (permalink / raw)
  To: Matthias May
  Cc: netdev, Florian Fainelli, Vivien Didelot, Woojung.Huh, jbe,
	sean.wang, john
In-Reply-To: <edf02eb7-e6bf-e88c-f655-40f6f71a3446@neratec.com>

On Wed, Sep 06, 2017 at 04:46:51PM +0200, Matthias May wrote:
> 
> Hi Andrew
> We are using the 88E6321.
> In our setup we are using openvswitch and not a bridge, however the problem you describe seems to be the same.
> 
> We had to configure the switch to flood unknown multicast (Egress Floods = 0x3, bits 3:2, offset 0x4 in port control)
> and
> unset FloodBC (FloodBC = 0x0, bit 12, offset 0x5 in global 2) which defines if a broadcast should be considered as
> multicast for the above config.

Hi Matthias

I might look at this.

But architecturally it seems better to add an mdb entry for the
broadcast address. I hope that work across all switches, where as what
you suggests only works for Marvell devices.

    Andrew

^ permalink raw reply

* Re: [PATCH 6/10] ixgbe: Use ARRAY_SIZE macro
From: Joe Perches @ 2017-09-06 15:19 UTC (permalink / raw)
  To: Thomas Meyer, David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20170906090817.f7qir5okeeajfcsc@olymp>

On Wed, 2017-09-06 at 11:08 +0200, Thomas Meyer wrote:
> On Tue, Sep 05, 2017 at 02:22:05PM -0700, David Miller wrote:
> > nothing prevented you from CC:'ing the maintainer,
> > Jeff Kirscher.
[]
> That's the other condition in my script. I only send to the role
> "maintainer" from the output of get_maintainer.pl. But Mr Jeff
> Kirscher is only listed as supporter...

Supporter means he gets paid to look after that
subsystem so he is something other than a volunteer.

from MAINTAINERS:
	S: Status, one of the following:
	   Supported:	Someone is actually paid to look after this.
	   Maintained:	Someone actually looks after it.
	   Odd Fixes:	It has a maintainer but they don't have time to do
			much other than throw the odd patch in. See below..
	   Orphan:	No current maintainer [but maybe you could take the
			role as you write your new code].
	   Obsolete:	Old code. Something tagged obsolete generally means
			it has been replaced by a better system and you
			should be using that.

^ permalink raw reply

* [PATCH net-next 0/1] netvsc hot plug fix
From: Stephen Hemminger @ 2017-09-06 15:19 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev

This is one fix for problems with hot add of network device
on Windows Server.

Stephen Hemminger (1):
  hv_netvsc: fix deadlock on hotplug

 drivers/net/hyperv/hyperv_net.h   |   1 +
 drivers/net/hyperv/netvsc_drv.c   |   8 +--
 drivers/net/hyperv/rndis_filter.c | 106 ++++++++++++++++++++++++++------------
 3 files changed, 74 insertions(+), 41 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH net-next 1/1] hv_netvsc: fix deadlock on hotplug
From: Stephen Hemminger @ 2017-09-06 15:19 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170906151925.15221-1-sthemmin@microsoft.com>

When a virtual device is added dynamically (via host console), then
the vmbus sends an offer message for the primary channel. The processing
of this message for networking causes the network device to then
initialize the sub channels.

The problem is that setting up the sub channels needs to wait until
the subsequent subchannel offers have been processed. These offers
come in on the same ring buffer and work queue as where the primary
offer is being processed; leading to a deadlock.

This did not happen in older kernels, because the sub channel waiting
logic was broken (it wasn't really waiting).

The solution is to do the sub channel setup in its own work queue
context that is scheduled by the primary channel setup; and then
happens later.

Fixes: 732e49850c5e ("netvsc: fix race on sub channel creation")
Reported-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
Should also go to stable, but this version does not apply cleanly
to 4.13. Have another patch for that.

 drivers/net/hyperv/hyperv_net.h   |   1 +
 drivers/net/hyperv/netvsc_drv.c   |   8 +--
 drivers/net/hyperv/rndis_filter.c | 106 ++++++++++++++++++++++++++------------
 3 files changed, 74 insertions(+), 41 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index ec546da86683..d1df5050adfb 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -172,6 +172,7 @@ struct rndis_device {
 	struct list_head req_list;
 
 	struct work_struct mcast_work;
+	struct work_struct subchan_work;
 
 	bool link_state;        /* 0 - link up, 1 - link down */
 
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 165ba4b3b423..80f45e0f81c8 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -853,10 +853,7 @@ static int netvsc_set_channels(struct net_device *net,
 	rndis_filter_device_remove(dev, nvdev);
 
 	nvdev = rndis_filter_device_add(dev, &device_info);
-	if (!IS_ERR(nvdev)) {
-		netif_set_real_num_tx_queues(net, nvdev->num_chn);
-		netif_set_real_num_rx_queues(net, nvdev->num_chn);
-	} else {
+	if (IS_ERR(nvdev)) {
 		ret = PTR_ERR(nvdev);
 		device_info.num_chn = orig;
 		nvdev = rndis_filter_device_add(dev, &device_info);
@@ -1954,9 +1951,6 @@ static int netvsc_probe(struct hv_device *dev,
 		NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
 	net->vlan_features = net->features;
 
-	netif_set_real_num_tx_queues(net, nvdev->num_chn);
-	netif_set_real_num_rx_queues(net, nvdev->num_chn);
-
 	netdev_lockdep_set_classes(net);
 
 	/* MTU range: 68 - 1500 or 65521 */
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 69c40b8fccc3..c240125eed48 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -33,6 +33,7 @@
 #include "hyperv_net.h"
 
 static void rndis_set_multicast(struct work_struct *w);
+static void rndis_set_subchannel(struct work_struct *w);
 
 #define RNDIS_EXT_LEN PAGE_SIZE
 struct rndis_request {
@@ -79,6 +80,7 @@ static struct rndis_device *get_rndis_device(void)
 
 	INIT_LIST_HEAD(&device->req_list);
 	INIT_WORK(&device->mcast_work, rndis_set_multicast);
+	INIT_WORK(&device->subchan_work, rndis_set_subchannel);
 
 	device->state = RNDIS_DEV_UNINITIALIZED;
 
@@ -1002,6 +1004,7 @@ static int rndis_filter_close_device(struct rndis_device *dev)
 
 	/* Make sure rndis_set_multicast doesn't re-enable filter! */
 	cancel_work_sync(&dev->mcast_work);
+	cancel_work_sync(&dev->subchan_work);
 
 	ret = rndis_filter_set_packet_filter(dev, 0);
 	if (ret == -ENODEV)
@@ -1054,6 +1057,71 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
 	wake_up(&nvscdev->subchan_open);
 }
 
+/*
+ * Open sub-channels after completing the handling of the device probe.
+ * This breaks overlap of processing the host message for the
+ * new primary channel with the initialization of sub-channels.
+ */
+static void rndis_set_subchannel(struct work_struct *w)
+{
+	struct rndis_device *rdev
+		= container_of(w, struct rndis_device, subchan_work);
+	struct net_device *net = rdev->ndev;
+	struct net_device_context *ndev_ctx = netdev_priv(net);
+	struct hv_device *dev = ndev_ctx->device_ctx;
+	struct nvsp_message *init_packet;
+	struct netvsc_device *nvdev;
+	int ret = -ENODEV;
+
+	if (!rtnl_trylock()) {
+		schedule_work(w);
+		return;
+	}
+
+	nvdev = rtnl_dereference(ndev_ctx->nvdev);
+	if (!nvdev)
+		goto out;
+
+	init_packet = &nvdev->channel_init_pkt;
+	memset(init_packet, 0, sizeof(struct nvsp_message));
+	init_packet->hdr.msg_type = NVSP_MSG5_TYPE_SUBCHANNEL;
+	init_packet->msg.v5_msg.subchn_req.op = NVSP_SUBCHANNEL_ALLOCATE;
+	init_packet->msg.v5_msg.subchn_req.num_subchannels =
+						nvdev->num_chn - 1;
+	ret = vmbus_sendpacket(dev->channel, init_packet,
+			       sizeof(struct nvsp_message),
+			       (unsigned long)init_packet,
+			       VM_PKT_DATA_INBAND,
+			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+	if (ret)
+		goto out;
+
+	wait_for_completion(&nvdev->channel_init_wait);
+	if (init_packet->msg.v5_msg.subchn_comp.status != NVSP_STAT_SUCCESS) {
+		netdev_err(net, "sub channel request failed\n");
+		goto out;
+	}
+
+	nvdev->num_chn = 1 +
+		init_packet->msg.v5_msg.subchn_comp.num_subchannels;
+
+	/* wait for all sub channels to open */
+	wait_event(nvdev->subchan_open,
+		   atomic_read(&nvdev->open_chn) == nvdev->num_chn);
+
+	/* ignore failues from setting rss parameters, still have channels */
+	rndis_filter_set_rss_param(rdev, netvsc_hash_key);
+out:
+	if (ret) {
+		nvdev->max_chn = 1;
+		nvdev->num_chn = 1;
+	}
+
+	netif_set_real_num_tx_queues(net, nvdev->num_chn);
+	netif_set_real_num_rx_queues(net, nvdev->num_chn);
+	rtnl_unlock();
+}
+
 struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
 				      struct netvsc_device_info *device_info)
 {
@@ -1063,7 +1131,6 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
 	struct rndis_device *rndis_device;
 	struct ndis_offload hwcaps;
 	struct ndis_offload_params offloads;
-	struct nvsp_message *init_packet;
 	struct ndis_recv_scale_cap rsscap;
 	u32 rsscap_size = sizeof(struct ndis_recv_scale_cap);
 	unsigned int gso_max_size = GSO_MAX_SIZE;
@@ -1215,9 +1282,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
 							net_device->num_chn);
 
 	atomic_set(&net_device->open_chn, 1);
-
-	if (net_device->num_chn == 1)
-		return net_device;
+	vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
 
 	for (i = 1; i < net_device->num_chn; i++) {
 		ret = netvsc_alloc_recv_comp_ring(net_device, i);
@@ -1228,38 +1293,11 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
 		}
 	}
 
-	vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
-
-	init_packet = &net_device->channel_init_pkt;
-	memset(init_packet, 0, sizeof(struct nvsp_message));
-	init_packet->hdr.msg_type = NVSP_MSG5_TYPE_SUBCHANNEL;
-	init_packet->msg.v5_msg.subchn_req.op = NVSP_SUBCHANNEL_ALLOCATE;
-	init_packet->msg.v5_msg.subchn_req.num_subchannels =
-						net_device->num_chn - 1;
-	ret = vmbus_sendpacket(dev->channel, init_packet,
-			       sizeof(struct nvsp_message),
-			       (unsigned long)init_packet,
-			       VM_PKT_DATA_INBAND,
-			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-	if (ret)
-		goto out;
-
-	wait_for_completion(&net_device->channel_init_wait);
-	if (init_packet->msg.v5_msg.subchn_comp.status != NVSP_STAT_SUCCESS) {
-		ret = -ENODEV;
-		goto out;
-	}
-
-	net_device->num_chn = 1 +
-		init_packet->msg.v5_msg.subchn_comp.num_subchannels;
-
-	/* wait for all sub channels to open */
-	wait_event(net_device->subchan_open,
-		   atomic_read(&net_device->open_chn) == net_device->num_chn);
+	if (net_device->num_chn > 1)
+		schedule_work(&rndis_device->subchan_work);
 
-	/* ignore failues from setting rss parameters, still have channels */
-	rndis_filter_set_rss_param(rndis_device, netvsc_hash_key);
 out:
+	/* if unavailable, just proceed with one queue */
 	if (ret) {
 		net_device->max_chn = 1;
 		net_device->num_chn = 1;
-- 
2.11.0

^ permalink raw reply related

* Re: [iproute PATCH 1/6] utils: Implement strlcpy() and strlcat()
From: Stephen Hemminger @ 2017-09-06 15:25 UTC (permalink / raw)
  To: David Laight; +Cc: Phil Sutter, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD006EC26@AcuExch.aculab.com>

On Wed, 6 Sep 2017 13:59:27 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: 04 September 2017 19:25
> > On Mon, 4 Sep 2017 17:00:15 +0200
> > Phil Sutter <phil@nwl.cc> wrote:
> >   
> > > On Mon, Sep 04, 2017 at 02:49:20PM +0000, David Laight wrote:  
> > > > From: Phil Sutter  
> > > > > Sent: 01 September 2017 17:53
> > > > > By making use of strncpy(), both implementations are really simple so
> > > > > there is no need to add libbsd as additional dependency.
> > > > >  
> > > > ...  
> > > > > +
> > > > > +size_t strlcpy(char *dst, const char *src, size_t size)
> > > > > +{
> > > > > +	if (size) {
> > > > > +		strncpy(dst, src, size - 1);
> > > > > +		dst[size - 1] = '\0';
> > > > > +	}
> > > > > +	return strlen(src);
> > > > > +}  
> > > >
> > > > Except that isn't really strlcpy().
> > > > Better would be:
> > > > 	len = strlen(src) + 1;
> > > > 	if (len <= size)
> > > > 		memcpy(dst, src, len);
> > > > 	else if (size) {
> > > > 		dst[size - 1] = 0;
> > > > 		memcpy(dst, src, size - 1);
> > > > 	}
> > > > 	return len - 1;  
> > >
> > > Please elaborate: Why isn't my version "really" strlcpy()? Why is your
> > > proposed version better?
> > >
> > > Thanks, Phil  
> > 
> > Linux kernel:
> > size_t strlcpy(char *dest, const char *src, size_t size)
> > {
> > 	size_t ret = strlen(src);
> > 
> > 	if (size) {
> > 		size_t len = (ret >= size) ? size - 1 : ret;
> > 		memcpy(dest, src, len);
> > 		dest[len] = '\0';
> > 	}
> > 	return ret;
> > }
> > 
> > FreeBSD:
> > size_t
> > strlcpy(char * __restrict dst, const char * __restrict src, size_t dsize)
> > {
> > 	const char *osrc = src;
> > 	size_t nleft = dsize;
> > 
> > 	/* Copy as many bytes as will fit. */
> > 	if (nleft != 0) {
> > 		while (--nleft != 0) {
> > 			if ((*dst++ = *src++) == '\0')
> > 				break;
> > 		}
> > 	}
> > 
> > 	/* Not enough room in dst, add NUL and traverse rest of src. */
> > 	if (nleft == 0) {
> > 		if (dsize != 0)
> > 			*dst = '\0';		/* NUL-terminate dst */
> > 		while (*src++)
> > 			;
> > 	}
> > 
> > 	return(src - osrc - 1);	/* count does not include NUL */
> > }
> > 
> > 
> > They all give the same results for some basic tests.
> > Test			FreeBSD		Linux		Iproute2
> > "",0:           	0 "JUNK"      	0 "JUNK"      	0 "JUNK"
> > "",1:           	0 ""          	0 ""          	0 ""
> > "",8:           	0 ""          	0 ""          	0 ""
> > "foo",0:        	3 "JUNK"      	3 "JUNK"      	3 "JUNK"
> > "foo",3:        	3 "fo"        	3 "fo"        	3 "fo"
> > "foo",4:        	3 "foo"       	3 "foo"       	3 "foo"
> > "foo",8:        	3 "foo"       	3 "foo"       	3 "foo"
> > "longstring",0: 	10 "JUNK"     	10 "JUNK"     	10 "JUNK"
> > "longstring",8: 	10 "longstr"  	10 "longstr"  	10 "longstr"  
> 
> You need to look at the contents of the destination buffer after the
> first '\0'.
> strlcpy() shouldn't change it.
> 
> 	David

Zeroing the bytes after the first null character should not be a big issue
other than a few nanoseconds extra work.

^ permalink raw reply

* [PATCH net-next] xdp: implement xdp_redirect_map for generic XDP
From: Jesper Dangaard Brouer @ 2017-09-06 15:26 UTC (permalink / raw)
  To: netdev, David S. Miller
  Cc: John Fastabend, Andy Gospodarek, Jesper Dangaard Brouer

Using bpf_redirect_map is allowed for generic XDP programs, but the
appropriate map lookup was never performed in xdp_do_generic_redirect().

Instead the map-index is directly used as the ifindex.  For the
xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
sending on ifindex 0 which isn't valid, resulting in getting SKB
packets dropped.  Thus, the reported performance numbers are wrong in
commit 24251c264798 ("samples/bpf: add option for native and skb mode
for redirect apps") for the 'xdp_redirect_map -S' case.

It might seem innocent this was lacking, but it can actually crash the
kernel.  The potential crash is caused by not consuming redirect_info->map.
The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map
pointer, which will survive even after unloading the xdp bpf_prog and
deallocating the devmap data-structure.  This leaves a dead map
pointer around.  The kernel will crash when loading the xdp_redirect
sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
and returns XDP_REDIRECT, which will cause it to dereference the map
pointer.

Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect apps")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 net/core/filter.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 5912c738a7b2..6a4745bf2c9f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2562,6 +2562,32 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
 }
 EXPORT_SYMBOL_GPL(xdp_do_redirect);
 
+static int xdp_do_generic_redirect_map(struct net_device *dev,
+				       struct sk_buff *skb,
+				       struct bpf_prog *xdp_prog)
+{
+	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_map *map = ri->map;
+	u32 index = ri->ifindex;
+	struct net_device *fwd;
+	int err;
+
+	ri->ifindex = 0;
+	ri->map = NULL;
+
+	fwd = __dev_map_lookup_elem(map, index);
+	if (!fwd) {
+		err = -EINVAL;
+		goto err;
+	}
+	skb->dev = fwd;
+	_trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index);
+	return 0;
+err:
+	_trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err);
+	return err;
+}
+
 int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 			    struct bpf_prog *xdp_prog)
 {
@@ -2571,6 +2597,9 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 	unsigned int len;
 	int err = 0;
 
+	if (ri->map)
+		return xdp_do_generic_redirect_map(dev, skb, xdp_prog);
+
 	fwd = dev_get_by_index_rcu(dev_net(dev), index);
 	ri->ifindex = 0;
 	if (unlikely(!fwd)) {

^ permalink raw reply related

* [net-next:master 491/511] xt_hashlimit.c:undefined reference to `__aeabi_uldivmod'
From: kbuild test robot @ 2017-09-06 15:27 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: kbuild-all, netdev

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

Hi Arnd,

It's probably a bug fix that unveils the link errors.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head:   66bed8465a808400eb14562510e26c8818082cb8
commit: 2c08ab3f2504bc7ba816ce6fde051b8bd5f028e4 [491/511] soc: ti/knav_dma: include dmaengine header
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 2c08ab3f2504bc7ba816ce6fde051b8bd5f028e4
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   net/netfilter/xt_hashlimit.o: In function `hashlimit_mt_common':
>> xt_hashlimit.c:(.text+0x1f68): undefined reference to `__aeabi_uldivmod'

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

^ permalink raw reply

* Re: [PATCH v2 rfc 0/8] IGMP snooping for local traffic
From: Stephen Hemminger @ 2017-09-06 15:27 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Florian Fainelli, Vivien Didelot, Woojung.Huh, jbe,
	sean.wang, john
In-Reply-To: <20170906004703.GB27385@lunn.ch>

On Wed, 6 Sep 2017 02:47:03 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > The third and last issue will be explained in a followup email.  
> 
> Hi DSA hackers
> 
> So there is the third issue. It affects just DSA, but it possible
> affects all DSA drivers.
> 
> This patchset broken broadcast with the Marvell drivers. It could
> break broadcast on others drivers as well.
> 
> What i found is that the Marvell chips don't flood broadcast frames
> between bridged ports. What appears to happen is there is a fdb miss,
> so it gets forwarded to the CPU port for the host to deal with. The
> software bridge when floods it out all ports of the bridge.
> 

That sounds like a good feature. There environments where you want
to disable broadcast between certain ports. It lets the bridge get
at the traffic for firewall filtering.

^ permalink raw reply

* Re: [PATCH v2 rfc 0/8] IGMP snooping for local traffic
From: Vivien Didelot @ 2017-09-06 15:25 UTC (permalink / raw)
  To: Andrew Lunn, netdev; +Cc: jiri, nikolay, Florian Fainelli, Andrew Lunn
In-Reply-To: <1504654510-31004-1-git-send-email-andrew@lunn.ch>

Hi Andrew, Nikolay,

Andrew Lunn <andrew@lunn.ch> writes:

> Then starts the work passing down to the hardware that the host has
> joined/left a group. The existing switchdev mdb object cannot be used,
> since the semantics are different. The existing
> SWITCHDEV_OBJ_ID_PORT_MDB is used to indicate a specific multicast
> group should be forwarded out that port of the switch. However here we
> require the exact opposite. We want multicast frames for the group
> received on the port to the forwarded to the host. Hence add a new
> object SWITCHDEV_OBJ_ID_HOST_MDB, a multicast database entry to
> forward to the host. This new object is then propagated through the
> DSA layers. No DSA driver changes should be needed, this should just
> work...

I'm not sure if you already explained that, if so, sorry in advance.

I don't understand why SWITCHDEV_OBJ_ID_PORT_MDB cannot be used. Isn't
setting the obj->orig_dev to the bridge device itself enough to
distinguish br0 from a switch port?

This way, the only change necessary in net/dsa/slave.c is something
like (abbreviated):

    static int dsa_slave_port_obj_add(struct net_device *dev,
                                    const struct switchdev_obj *obj,
                                    struct switchdev_trans *trans)
    {
            struct dsa_slave_priv *p = netdev_priv(dev);
            struct dsa_port *port = p->dp;

            /* Is the target port the bridge device itself? */
            if (obj->orig_dev == port->br)
                    port = port->cpu_dp;

            return dsa_port_mdb_add(port, obj, trans);
    }

The main problem is that we will soon want support for multiple CPU
ports. This means that each DSA slave will have its dedicated CPU port,
instead of having only one for the whole switch tree.

So adding the MDB entry to all CPU ports as you did in a patch 5/8 in
dsa_switch_host_mdb_*() is not correct because you can have CPU port
sw0p0 being a member of br0 and CPU port sw0p10 being a member of br1.

So is it correct to simply notify SWITCHDEV_OBJ_ID_PORT_MDB with
orig_dev = br->dev so that its members get recursively notified?

Even if SWITCHDEV_OBJ_ID_HOST_MDB is necessary, we need to handle it the
way I described, otherwise we don't have a correct mapping between a
slave port and its CPU port that we need to configure.


Thanks,

        Vivien

^ permalink raw reply

* [PATCH net] rds: Fix incorrect statistics counting
From: Håkon Bugge @ 2017-09-06 15:29 UTC (permalink / raw)
  To: Santosh Shilimkar, David S . Miller
  Cc: netdev, linux-rdma, rds-devel, linux-kernel, knut.omang

In rds_send_xmit() there is logic to batch the sends. However, if
another thread has acquired the lock, it is considered a race and we
yield. The code incrementing the s_send_lock_queue_raced statistics
counter did not count this event correctly.

This commit removes a small race in determining the race and
increments the statistics counter correctly.

Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
Reviewed-by: Knut Omang <knut.omang@oracle.com>
---
 net/rds/send.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/rds/send.c b/net/rds/send.c
index 058a407..ecfe0b5 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -101,6 +101,11 @@ void rds_send_path_reset(struct rds_conn_path *cp)
 }
 EXPORT_SYMBOL_GPL(rds_send_path_reset);
 
+static bool someone_in_xmit(struct rds_conn_path *cp)
+{
+	return test_bit(RDS_IN_XMIT, &cp->cp_flags);
+}
+
 static int acquire_in_xmit(struct rds_conn_path *cp)
 {
 	return test_and_set_bit(RDS_IN_XMIT, &cp->cp_flags) == 0;
@@ -428,14 +433,19 @@ int rds_send_xmit(struct rds_conn_path *cp)
 	 * some work and we will skip our goto
 	 */
 	if (ret == 0) {
+		bool raced;
+
 		smp_mb();
+		raced = someone_in_xmit(cp) ||
+			send_gen != READ_ONCE(cp->cp_send_gen);
+
 		if ((test_bit(0, &conn->c_map_queued) ||
-		     !list_empty(&cp->cp_send_queue)) &&
-			send_gen == READ_ONCE(cp->cp_send_gen)) {
-			rds_stats_inc(s_send_lock_queue_raced);
+				!list_empty(&cp->cp_send_queue)) && !raced) {
 			if (batch_count < send_batch_count)
 				goto restart;
 			queue_delayed_work(rds_wq, &cp->cp_send_w, 1);
+		} else if (raced) {
+			rds_stats_inc(s_send_lock_queue_raced);
 		}
 	}
 out:
-- 
2.9.3

^ permalink raw reply related

* Fw: [Bug 196839] New: use_time of IPsec policy is updated even when receiving error packets.
From: Stephen Hemminger @ 2017-09-06 15:38 UTC (permalink / raw)
  To: netdev



Begin forwarded message:

Date: Wed, 06 Sep 2017 10:18:33 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 196839] New: use_time of IPsec policy is updated even when receiving error packets.


https://bugzilla.kernel.org/show_bug.cgi?id=196839

            Bug ID: 196839
           Summary: use_time of IPsec policy is updated even when
                    receiving error packets.
           Product: Networking
           Version: 2.5
    Kernel Version: 4.8.0
          Hardware: Intel
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Other
          Assignee: stephen@networkplumber.org
          Reporter: cchenme@gmail.com
        Regression: No

Normally the use_time of policy in SPD is updated if the policy is matched by
incoming or outgoing IP packets. For protect policy, it is updated if ESP/AH
packets are sent or received.

The use_time of SPD_IN policy used by IKE implementation like strongSwan to
check whether there is inbound traffic, thus determine whether it is necessary
to send DPD(dead peer detection, rfc3706) request to check liveness of IPsec
peer.

In case an unprotected packet is received but matches the IPsec SPD IN protect
policy, the packet will be discarded and the error counter XfrmInTmplMismatch
in /proc/net/xfrm_stat is incremented.

But in such error/malicious case, the use_time of SPD IN policy is also
updated. This cause strongSwan to mistakenly regard that the policy is in use
and not to trigger DPD request even when it should.

In short, this is a security hole in kernel and could lead to DoS attack on
IPsec gateway running on Linux.

-- 
You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* Re: [PATCH v2 rfc 4/8] net: dsa: slave: Handle switchdev host mdb add/del
From: Vivien Didelot @ 2017-09-06 15:37 UTC (permalink / raw)
  To: Andrew Lunn, netdev; +Cc: jiri, nikolay, Florian Fainelli, Andrew Lunn
In-Reply-To: <1504654510-31004-5-git-send-email-andrew@lunn.ch>

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> Add code to handle switchdev host mdb add/del. As with normal mdb
> add/del, send a notification to the switch layer.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  net/dsa/dsa_priv.h |  7 +++++++
>  net/dsa/port.c     | 26 ++++++++++++++++++++++++++
>  net/dsa/slave.c    |  6 ++++++
>  3 files changed, 39 insertions(+)
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 9c3eeb72462d..0ffe49f78d14 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -24,6 +24,8 @@ enum {
>  	DSA_NOTIFIER_FDB_DEL,
>  	DSA_NOTIFIER_MDB_ADD,
>  	DSA_NOTIFIER_MDB_DEL,
> +	DSA_NOTIFIER_HOST_MDB_ADD,
> +	DSA_NOTIFIER_HOST_MDB_DEL,
>  	DSA_NOTIFIER_VLAN_ADD,
>  	DSA_NOTIFIER_VLAN_DEL,
>  };
> @@ -131,6 +133,11 @@ int dsa_port_mdb_add(struct dsa_port *dp,
>  		     struct switchdev_trans *trans);
>  int dsa_port_mdb_del(struct dsa_port *dp,
>  		     const struct switchdev_obj_port_mdb *mdb);
> +int dsa_host_mdb_add(struct dsa_port *dp,
> +		     const struct switchdev_obj_port_mdb *mdb,
> +		     struct switchdev_trans *trans);
> +int dsa_host_mdb_del(struct dsa_port *dp,
> +		     const struct switchdev_obj_port_mdb *mdb);
>  int dsa_port_vlan_add(struct dsa_port *dp,
>  		      const struct switchdev_obj_port_vlan *vlan,
>  		      struct switchdev_trans *trans);
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 659676ba3f8b..5b18b9fe2219 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -199,6 +199,32 @@ int dsa_port_mdb_del(struct dsa_port *dp,
>  	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_DEL, &info);
>  }
>  
> +int dsa_host_mdb_add(struct dsa_port *dp,
> +		     const struct switchdev_obj_port_mdb *mdb,
> +		     struct switchdev_trans *trans)
> +{
> +	struct dsa_notifier_mdb_info info = {
> +		.sw_index = dp->ds->index,
> +		.port = dp->index,
> +		.trans = trans,
> +		.mdb = mdb,
> +	};
> +
> +	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_ADD, &info);
> +}
> +
> +int dsa_host_mdb_del(struct dsa_port *dp,
> +		     const struct switchdev_obj_port_mdb *mdb)
> +{
> +	struct dsa_notifier_mdb_info info = {
> +		.sw_index = dp->ds->index,
> +		.port = dp->index,
> +		.mdb = mdb,
> +	};
> +
> +	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_DEL, &info);
> +}
> +
>  int dsa_port_vlan_add(struct dsa_port *dp,
>  		      const struct switchdev_obj_port_vlan *vlan,
>  		      struct switchdev_trans *trans)
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 78e78a6e6833..2e07be149415 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -330,6 +330,9 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
>  	case SWITCHDEV_OBJ_ID_PORT_MDB:
>  		err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans);
>  		break;
> +	case SWITCHDEV_OBJ_ID_HOST_MDB:
> +		err = dsa_host_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans);
> +		break;

If SWITCHDEV_OBJ_ID_HOST_MDB is really necessary, 

    case SWITCHDEV_OBJ_ID_HOST_MDB:
        err = dsa_port_mdb_add(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans);
        break;

should be enough. DSA_NOTIFIER_HOST_MDB_* are not necessary.


Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH net-next] xdp: implement xdp_redirect_map for generic XDP
From: Andy Gospodarek @ 2017-09-06 15:44 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: netdev@vger.kernel.org, David S. Miller, John Fastabend
In-Reply-To: <150471158528.3727.12324542627400287360.stgit@firesoul>

On Wed, Sep 6, 2017 at 11:26 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Using bpf_redirect_map is allowed for generic XDP programs, but the
> appropriate map lookup was never performed in xdp_do_generic_redirect().
>
> Instead the map-index is directly used as the ifindex.  For the
> xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
> sending on ifindex 0 which isn't valid, resulting in getting SKB
> packets dropped.  Thus, the reported performance numbers are wrong in
> commit 24251c264798 ("samples/bpf: add option for native and skb mode
> for redirect apps") for the 'xdp_redirect_map -S' case.
>
> It might seem innocent this was lacking, but it can actually crash the
> kernel.  The potential crash is caused by not consuming redirect_info->map.
> The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map
> pointer, which will survive even after unloading the xdp bpf_prog and
> deallocating the devmap data-structure.  This leaves a dead map
> pointer around.  The kernel will crash when loading the xdp_redirect
> sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
> and returns XDP_REDIRECT, which will cause it to dereference the map
> pointer.

Nice catch!

Since 'net-next' is closed and this is a bugfix it seems like this is
a good candidate for 'net' right?

>
> Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
> Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect apps")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

Acked-by: Andy Gospodarek <andy@greyhouse.net>


> ---
>  net/core/filter.c |   29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 5912c738a7b2..6a4745bf2c9f 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2562,6 +2562,32 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
>  }
>  EXPORT_SYMBOL_GPL(xdp_do_redirect);
>
> +static int xdp_do_generic_redirect_map(struct net_device *dev,
> +                                      struct sk_buff *skb,
> +                                      struct bpf_prog *xdp_prog)
> +{
> +       struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> +       struct bpf_map *map = ri->map;
> +       u32 index = ri->ifindex;
> +       struct net_device *fwd;
> +       int err;
> +
> +       ri->ifindex = 0;
> +       ri->map = NULL;
> +
> +       fwd = __dev_map_lookup_elem(map, index);
> +       if (!fwd) {
> +               err = -EINVAL;
> +               goto err;
> +       }
> +       skb->dev = fwd;
> +       _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index);
> +       return 0;
> +err:
> +       _trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err);
> +       return err;
> +}
> +
>  int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
>                             struct bpf_prog *xdp_prog)
>  {
> @@ -2571,6 +2597,9 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
>         unsigned int len;
>         int err = 0;
>
> +       if (ri->map)
> +               return xdp_do_generic_redirect_map(dev, skb, xdp_prog);
> +
>         fwd = dev_get_by_index_rcu(dev_net(dev), index);
>         ri->ifindex = 0;
>         if (unlikely(!fwd)) {
>

^ permalink raw reply

* Re: hung task in mac80211
From: Sebastian Gottschall @ 2017-09-06 15:45 UTC (permalink / raw)
  To: Matteo Croce, Johannes Berg; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <CAGnkfhyBMKT-Tf10NxjN40Znuj7OK2pSM3qLUidPSkQqh8gjwQ@mail.gmail.com>

i confirm this patch fixes the issue for me as well

Am 06.09.2017 um 17:04 schrieb Matteo Croce:
> On Wed, Sep 6, 2017 at 2:58 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Wed, 2017-09-06 at 13:57 +0200, Matteo Croce wrote:
>>
>>> I have an hung task on vanilla 4.13 kernel which I haven't on 4.12.
>>> The problem is present both on my AP and on my notebook,
>>> so it seems it affects AP and STA mode as well.
>>> The generated messages are:
>>>
>>> INFO: task kworker/u16:6:120 blocked for more than 120 seconds.
>>>        Not tainted 4.13.0 #57
>>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
>>> message.
>>> kworker/u16:6   D    0   120      2 0x00000000
>>> Workqueue: phy0 ieee80211_ba_session_work [mac80211]
>>> Call Trace:
>>>   ? __schedule+0x174/0x5b0
>>>   ? schedule+0x31/0x80
>>>   ? schedule_preempt_disabled+0x9/0x10
>>>   ? __mutex_lock.isra.2+0x163/0x480
>>>   ? select_task_rq_fair+0xb9f/0xc60
>>>   ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>>>   ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>> Yeah - obviously as Stefano found, both take &sta->ampdu_mlme.mtx.
>>
>> Can you try this?
>>
>> diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
>> index 2b36eff5d97e..d8d32776031e 100644
>> --- a/net/mac80211/agg-rx.c
>> +++ b/net/mac80211/agg-rx.c
>> @@ -245,10 +245,10 @@ static void ieee80211_send_addba_resp(struct ieee80211_sub_if_data *sdata, u8 *d
>>          ieee80211_tx_skb(sdata, skb);
>>   }
>>
>> -void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>> -                                    u8 dialog_token, u16 timeout,
>> -                                    u16 start_seq_num, u16 ba_policy, u16 tid,
>> -                                    u16 buf_size, bool tx, bool auto_seq)
>> +void ___ieee80211_start_rx_ba_session(struct sta_info *sta,
>> +                                     u8 dialog_token, u16 timeout,
>> +                                     u16 start_seq_num, u16 ba_policy, u16 tid,
>> +                                     u16 buf_size, bool tx, bool auto_seq)
>>   {
>>          struct ieee80211_local *local = sta->sdata->local;
>>          struct tid_ampdu_rx *tid_agg_rx;
>> @@ -267,7 +267,7 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>>                  ht_dbg(sta->sdata,
>>                         "STA %pM requests BA session on unsupported tid %d\n",
>>                         sta->sta.addr, tid);
>> -               goto end_no_lock;
>> +               goto end;
>>          }
>>
>>          if (!sta->sta.ht_cap.ht_supported) {
>> @@ -275,14 +275,14 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>>                         "STA %pM erroneously requests BA session on tid %d w/o QoS\n",
>>                         sta->sta.addr, tid);
>>                  /* send a response anyway, it's an error case if we get here */
>> -               goto end_no_lock;
>> +               goto end;
>>          }
>>
>>          if (test_sta_flag(sta, WLAN_STA_BLOCK_BA)) {
>>                  ht_dbg(sta->sdata,
>>                         "Suspend in progress - Denying ADDBA request (%pM tid %d)\n",
>>                         sta->sta.addr, tid);
>> -               goto end_no_lock;
>> +               goto end;
>>          }
>>
>>          /* sanity check for incoming parameters:
>> @@ -296,7 +296,7 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>>                  ht_dbg_ratelimited(sta->sdata,
>>                                     "AddBA Req with bad params from %pM on tid %u. policy %d, buffer size %d\n",
>>                                     sta->sta.addr, tid, ba_policy, buf_size);
>> -               goto end_no_lock;
>> +               goto end;
>>          }
>>          /* determine default buffer size */
>>          if (buf_size == 0)
>> @@ -311,7 +311,6 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>>                 buf_size, sta->sta.addr);
>>
>>          /* examine state machine */
>> -       mutex_lock(&sta->ampdu_mlme.mtx);
>>
>>          if (test_bit(tid, sta->ampdu_mlme.agg_session_valid)) {
>>                  if (sta->ampdu_mlme.tid_rx_token[tid] == dialog_token) {
>> @@ -415,15 +414,25 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>>                  __clear_bit(tid, sta->ampdu_mlme.unexpected_agg);
>>                  sta->ampdu_mlme.tid_rx_token[tid] = dialog_token;
>>          }
>> -       mutex_unlock(&sta->ampdu_mlme.mtx);
>>
>> -end_no_lock:
>>          if (tx)
>>                  ieee80211_send_addba_resp(sta->sdata, sta->sta.addr, tid,
>>                                            dialog_token, status, 1, buf_size,
>>                                            timeout);
>>   }
>>
>> +void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>> +                                    u8 dialog_token, u16 timeout,
>> +                                    u16 start_seq_num, u16 ba_policy, u16 tid,
>> +                                    u16 buf_size, bool tx, bool auto_seq)
>> +{
>> +       mutex_lock(&sta->ampdu_mlme.mtx);
>> +       ___ieee80211_start_rx_ba_session(sta, dialog_token, timeout,
>> +                                        start_seq_num, ba_policy, tid,
>> +                                        buf_size, tx, auto_seq);
>> +       mutex_unlock(&sta->ampdu_mlme.mtx);
>> +}
>> +
>>   void ieee80211_process_addba_request(struct ieee80211_local *local,
>>                                       struct sta_info *sta,
>>                                       struct ieee80211_mgmt *mgmt,
>> diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
>> index c92df492e898..198b2d3e56fd 100644
>> --- a/net/mac80211/ht.c
>> +++ b/net/mac80211/ht.c
>> @@ -333,9 +333,9 @@ void ieee80211_ba_session_work(struct work_struct *work)
>>
>>                  if (test_and_clear_bit(tid,
>>                                         sta->ampdu_mlme.tid_rx_manage_offl))
>> -                       __ieee80211_start_rx_ba_session(sta, 0, 0, 0, 1, tid,
>> -                                                       IEEE80211_MAX_AMPDU_BUF,
>> -                                                       false, true);
>> +                       ___ieee80211_start_rx_ba_session(sta, 0, 0, 0, 1, tid,
>> +                                                        IEEE80211_MAX_AMPDU_BUF,
>> +                                                        false, true);
>>
>>                  if (test_and_clear_bit(tid + IEEE80211_NUM_TIDS,
>>                                         sta->ampdu_mlme.tid_rx_manage_offl))
>> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
>> index 2197c62a0a6e..9675814f64db 100644
>> --- a/net/mac80211/ieee80211_i.h
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -1760,6 +1760,10 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
>>                                       u8 dialog_token, u16 timeout,
>>                                       u16 start_seq_num, u16 ba_policy, u16 tid,
>>                                       u16 buf_size, bool tx, bool auto_seq);
>> +void ___ieee80211_start_rx_ba_session(struct sta_info *sta,
>> +                                     u8 dialog_token, u16 timeout,
>> +                                     u16 start_seq_num, u16 ba_policy, u16 tid,
>> +                                     u16 buf_size, bool tx, bool auto_seq);
>>   void ieee80211_sta_tear_down_BA_sessions(struct sta_info *sta,
>>                                           enum ieee80211_agg_stop_reason reason);
>>   void ieee80211_process_delba(struct ieee80211_sub_if_data *sdata,
>>
>> johannes
> I confirm that this patch fixes the hang too.
> I'm curious to see if there are noticeable performance differences
> between the two solutions.
>
> Cheers,


-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Berliner Ring 101, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

^ permalink raw reply

* Re: VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)
From: Sergey Matyukevich @ 2017-09-06 15:45 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev, Igor Mitsyanko,
	Avinash Patil
In-Reply-To: <1504621233.12380.21.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>


Hi Johannes and all,

> > In a way this feature seems mis-designed - you never have 802.1Q tags
> > over the air, but you're inserting them on RX and stripping them on
> > TX, probably in order to make bridging to ethernet easier and not
> > have to have 802.1Q acceleration on the ethernet port, or - well - in
> > order to have an ability to do this with an ethernet card that only
> > has a single CPU port.
> 
> Ok this isn't really right either - it's only for saving the 802.1Q
> acceleration on the Ethernet port, really - and saving the extra
> bridges.
> 
> To clarify, I think what you - conceptually - want is the following
> topology:
> 
>         +--- eth0.1  ---  br.1  ---  wlan0.1
>         |
> eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
>         |
>         +--- eth0.3  ---  br.3  ---  wlan0.3
> 
> where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
> and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
> type interfaces that isolate the clients against each other as far as
> wifi is concerned.
> 
> Is this correct? As far as I understand, that's the baseline topology
> that you're trying to achieve, expressed in terms of Linux networking.

That's right. In fact, hostapd is able to create this kind of network
bridge infrastructure automatically when it is built
with CONFIG_FULL_DYNAMIC_VLAN option enabled.

> Now, you seem to want to compress this to
> 
>                   +---  wlan0.1
>                   |
> eth0  ---  br  ---+---  wlan0.2
>                   |
>                   +---  wlan0.3
> 
> and have the 802.1Q tag insertion/removal that's normally configured to
> happen in eth0.N already be handled in wlan0.N.
> 
> Also correct?

Exactly. And yes, the only purpose of this 'non-conventional' mode was
to have 802.1Q acceleration on the ethernet port.

> 
> 
> We clearly don't have APIs for this, and I don't think it makes sense
> in the Linux space - the bridge and wlan0.N suddenly have tagged
> traffic rather than untagged, and the VLAN tagging is completely hidden
> from the management view.
> 
> johannes

^ permalink raw reply

* RE: [PATCH v2 rfc 0/8] IGMP snooping for local traffic
From: Woojung.Huh @ 2017-09-06 15:49 UTC (permalink / raw)
  To: andrew, netdev, f.fainelli, vivien.didelot, jbe, sean.wang, john
In-Reply-To: <20170906004703.GB27385@lunn.ch>

Andrew,

> What i found is that the Marvell chips don't flood broadcast frames
> between bridged ports. What appears to happen is there is a fdb miss,
> so it gets forwarded to the CPU port for the host to deal with. The
> software bridge when floods it out all ports of the bridge.
Is this IGMP snooping enabled mode in Marvell chip?

^ permalink raw reply

* Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
From: Mason @ 2017-09-06 15:51 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: Marc Gonzalez, David Daney, netdev, Geert Uytterhoeven,
	David Miller, Mans Rullgard, Thibaud Cornic
In-Reply-To: <7b1c1dc9-b6e3-a1bd-2e36-474946741a79@gmail.com>

On 31/08/2017 21:18, Florian Fainelli wrote:

> On 08/31/2017 12:09 PM, Mason wrote:
>
>> On 31/08/2017 19:03, Florian Fainelli wrote:
>>
>>> On 08/31/2017 05:29 AM, Marc Gonzalez wrote:
>>>
>>>> On 31/08/2017 02:49, Florian Fainelli wrote:
>>>>
>>>>> The original motivation for this change originated from Marc Gonzalez
>>>>> indicating that his network driver did not have its adjust_link callback
>>>>> executing with phydev->link = 0 while he was expecting it.
>>>>
>>>> I expect the core to call phy_adjust_link() for link changes.
>>>> This used to work back in 3.4 and was broken somewhere along
>>>> the way.
>>>
>>> If that was working correctly in 3.4 surely we can look at the diff and
>>> figure out what changed, even maybe find the offending commit, can you
>>> do that?
>>
>> Bisecting would a be a huge pain because my platform was
>> not upstream until v4.4
> 
> Then just diff the file and try to pinpoint which commit may have
> changed that?

Running 'ip link set eth0 down' on the command-line.

In v3.4 => adjust_link() callback is called
In v4.5 => adjust_link() callback is NOT called

$ git log --oneline --no-merges v3.4..v4.5 drivers/net/phy/phy.c | wc -l
59

I'm not sure what "just diff the file" entails.
I can't move 3.4 up, nor move 4.5 down.
I'm not even sure the problem comes from drivers/net/phy/phy.c
to be honest.

Regards.

^ 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