Netdev List
 help / color / mirror / Atom feed
* iproute2 ss outputs duplicate tcp sockets info on kernel 3.10.105
From: Li Er @ 2017-05-09  1:56 UTC (permalink / raw)
  To: netdev

i'm using v4.11.0 release of iproute2 and kernel 3.10.105, simply
running

        $ ss
        Netid  State      Recv-Q Send-Q Local Address:Port                 Peer Address:Port
        tcp    CLOSE-WAIT 434    0      10.0.0.1:47931                65.49.18.136:https
        tcp    CLOSE-WAIT 432    0      10.0.0.1:47932                65.49.18.136:https
        tcp    CLOSE-WAIT 434    0      10.0.0.1:47931                65.49.18.136:https
        tcp    CLOSE-WAIT 432    0      10.0.0.1:47932                65.49.18.136:https

as you can see, there's one duplicate entry of each  tcp  socket,
however,  if  i  explicitly  specify  tcp socket by adding the -t
switch,

        $ ss -t
        State      Recv-Q Send-Q Local Address:Port                 Peer Address:Port
        CLOSE-WAIT 434    0      10.0.0.1:47931                65.49.18.136:https
        CLOSE-WAIT 432    0      10.0.0.1:47932                65.49.18.136:https

the duplication is gone.

this problem also occurs on  git  master,  but  not  on  iproute2
v4.3.0,  so  i  did  a  git bisect and found out the commit which
caused this is 9f66764e308e9c645b3fb2d1cfbb7fb207eb5de1,  and  by
revert this commit on git master, i.e. removing

                                        err = rtnl_dump_done(rth, h);
                                        if (err < 0)
                                                return -1;

these 3 lines of code of lib/libnetlink.c, the problem is gone.

since  i'm not familiar with the source code, i doubt this is the
right way to solve the problem, what's your suggestions? thanks.

^ permalink raw reply

* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: Eric Dumazet @ 2017-05-09  2:18 UTC (permalink / raw)
  To: David Miller; +Cc: xiyou.wangcong, netdev, andreyknvl, edumazet
In-Reply-To: <20170508.212211.1291611254198273979.davem@davemloft.net>

On Mon, 2017-05-08 at 21:22 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 08 May 2017 17:01:20 -0700
> 
> > On Mon, 2017-05-08 at 14:35 -0400, David Miller wrote:
> >> From: Cong Wang <xiyou.wangcong@gmail.com>
> >> Date: Thu,  4 May 2017 14:54:17 -0700
> >> 
> >> > IPv4 dst could use fi->fib_metrics to store metrics but fib_info
> >> > itself is refcnt'ed, so without taking a refcnt fi and
> >> > fi->fib_metrics could be freed while dst metrics still points to
> >> > it. This triggers use-after-free as reported by Andrey twice.
> >> > 
> >> > This patch reverts commit 2860583fe840 ("ipv4: Kill rt->fi") to
> >> > restore this reference counting. It is a quick fix for -net and
> >> > -stable, for -net-next, as Eric suggested, we can consider doing
> >> > reference counting for metrics itself instead of relying on fib_info.
> >> > 
> >> > IPv6 is very different, it copies or steals the metrics from mx6_config
> >> > in fib6_commit_metrics() so probably doesn't need a refcnt.
> >> > 
> >> > Decnet has already done the refcnt'ing, see dn_fib_semantic_match().
> >> > 
> >> > Fixes: 2860583fe840 ("ipv4: Kill rt->fi")
> >> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> >> > Tested-by: Andrey Konovalov <andreyknvl@google.com>
> >> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> >> 
> >> Applied and queued up for -stable, thanks.
> > 
> > Although I now have on latest net tree these messages when I reboot my
> > test machine.
> > 
> > [  224.085873] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
> 
> Strange, the refcounting looks quite OK in the patch you're quoting.
> I looked over it a few times and cannot figure out a possible cause
> there.
> 
> I am assuming you are quite confident it is this change?

At least, reverting the patch resolves the issue for me.

Keeping fib (and their reference to netdev) is apparently too much,
we probably need to implement a refcount on the metrics themselves,
being stand alone objects.

^ permalink raw reply

* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: David Miller @ 2017-05-09  2:35 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xiyou.wangcong, netdev, andreyknvl, edumazet
In-Reply-To: <1494296302.7796.61.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 08 May 2017 19:18:22 -0700

> Keeping fib (and their reference to netdev) is apparently too much,
> we probably need to implement a refcount on the metrics themselves,
> being stand alone objects.

I think I see the problem, when we flush the dst cache on device
unregister, we point the dst at loopback but do not flush metrics.

I'm going to revert.

^ permalink raw reply

* Re: [PATCH net] ip6_tunnel: remove unreachable ICMP_REDIRECT code
From: Hangbin Liu @ 2017-05-09  3:25 UTC (permalink / raw)
  To: David Miller; +Cc: network dev
In-Reply-To: <20170508.155903.61649135414142419.davem@davemloft.net>

2017-05-09 3:59 GMT+08:00 David Miller <davem@davemloft.net>:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Mon,  8 May 2017 19:11:03 +0800
>
>> After call ip6_tnl_err(), the rel_type will be ether ICMPV6_DEST_UNREACH
>> or ICMPV6_PKT_TOOBIG. We will never reach ICMP_REDIRECT. So remove it.
>>
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>
> Your patches here, all seemingly due to "visual inspection", are starting
> to really, truly, irritate me.

Yes, this is based on the coverity scan result.  I'm really sorry to make
you feel uncomfortable and I apologize for this.  I trust the result too much
and did not do enough research. I will try my best to make sure the patch
really improve or fix something next time.

Sorry again.

Regards
Hangbin

>
> Half of them are unnecessary, some are completely buggy.
>
> I have to review them and audit them very satrictly as a result, and
> this takes up an unnecessarily large amount of my time.
>
> Therefore, I'm basically going to stop looking at your changes _unless_
> you can show that you specifically tested and exercised all of the code
> paths you are changing.
>
> I am sorry to have to do this, but the value to effort ratio of
> reviewing and integrating your changes is quite poor.

^ permalink raw reply

* [PATCH] net: wireless: ath: ath9k: remove unnecessary code
From: Gustavo A. R. Silva @ 2017-05-09  3:48 UTC (permalink / raw)
  To: QCA ath9k Development, Kalle Valo
  Cc: linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva

The name of an array used by itself will always return the array's address.
So this test will always evaluate as true.

Addresses-Coverity-ID: 1364903
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/net/wireless/ath/ath9k/eeprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index fb80ec8..5c3bc28 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -143,7 +143,7 @@ bool ath9k_hw_nvram_read(struct ath_hw *ah, u32 off, u16 *data)
 
 	if (ah->eeprom_blob)
 		ret = ath9k_hw_nvram_read_firmware(ah->eeprom_blob, off, data);
-	else if (pdata && !pdata->use_eeprom && pdata->eeprom_data)
+	else if (pdata && !pdata->use_eeprom)
 		ret = ath9k_hw_nvram_read_pdata(pdata, off, data);
 	else
 		ret = common->bus_ops->eeprom_read(common, off, data);
-- 
2.5.0

^ permalink raw reply related

* [PATCH] net: wireless: ath: ath10k: remove unnecessary code
From: Gustavo A. R. Silva @ 2017-05-09  4:21 UTC (permalink / raw)
  To: Kalle Valo
  Cc: ath10k, linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva

The name of an array used by itself will always return the array's address.
So these tests will always evaluate as false and therefore the _return_
will never be executed.

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/net/wireless/ath/ath10k/wmi.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 2f1743e..135cf83 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -5933,15 +5933,6 @@ static struct sk_buff *ath10k_wmi_10_4_op_gen_init(struct ath10k *ar)
 
 int ath10k_wmi_start_scan_verify(const struct wmi_start_scan_arg *arg)
 {
-	if (arg->ie_len && !arg->ie)
-		return -EINVAL;
-	if (arg->n_channels && !arg->channels)
-		return -EINVAL;
-	if (arg->n_ssids && !arg->ssids)
-		return -EINVAL;
-	if (arg->n_bssids && !arg->bssids)
-		return -EINVAL;
-
 	if (arg->ie_len > WLAN_SCAN_PARAMS_MAX_IE_LEN)
 		return -EINVAL;
 	if (arg->n_channels > ARRAY_SIZE(arg->channels))
-- 
2.5.0

^ permalink raw reply related

* [PATCH] wcn36xx: Close SMD channel on device removal
From: Bjorn Andersson @ 2017-05-09  4:36 UTC (permalink / raw)
  To: Eugene Krasnikov, Kalle Valo, Eyal Ilsar
  Cc: wcn36xx, linux-wireless, netdev, linux-kernel, linux-arm-msm

The SMD channel is not the primary WCNSS channel and must explicitly be
closed as the device is removed, or the channel will already by open on
a subsequent probe call in e.g. the case of reloading the kernel module.

This issue was introduced because I simplified the underlying SMD
implementation while the SMD adaptions of the driver sat on the mailing
list, but missed to update these patches. The patch does however only
apply back to the transition to rpmsg, hence the limited Fixes.

Fixes: 5052de8deff5 ("soc: qcom: smd: Transition client drivers from smd to rpmsg")
Reported-by: Eyal Ilsar <c_eilsar@qti.qualcomm.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index d5e993dc9b23..517a315e259b 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1271,6 +1271,8 @@ static int wcn36xx_remove(struct platform_device *pdev)
 	qcom_smem_state_put(wcn->tx_enable_state);
 	qcom_smem_state_put(wcn->tx_rings_empty_state);
 
+	rpmsg_destroy_ept(wcn->smd_channel);
+
 	iounmap(wcn->dxe_base);
 	iounmap(wcn->ccu_base);
 
-- 
2.12.0

^ permalink raw reply related

* Re: [PATCH 0/4] TI Bluetooth serdev support
From: Baruch Siach @ 2017-05-09  4:48 UTC (permalink / raw)
  To: Rob Herring
  Cc: Adam Ford, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Johan Hedberg,
	Gustavo Padovan, Marcel Holtmann, Sebastian Reichel, Wei Xu,
	open list:BLUETOOTH DRIVERS, Eyal Reizer, netdev, Satish Patel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAL_JsqKBAPUPRtn+pEtz8B8pCrA=45RkEq6X_0i_HYoWsVmtyw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,

On Mon, May 08, 2017 at 04:12:16PM -0500, Rob Herring wrote:
> On Fri, May 5, 2017 at 9:51 AM, Adam Ford <aford173-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Sun, Apr 30, 2017 at 11:04 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> On Sun, Apr 30, 2017 at 10:14:20AM -0500, Adam Ford wrote:
> >>> On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >>> > This series adds serdev support to the HCI LL protocol used on TI BT
> >>> > modules and enables support on HiKey board with with the WL1835 module.
> >>> > With this the custom TI UIM daemon and btattach are no longer needed.
> >>>
> >>> Without UIM daemon, what instruction do you use to load the BT firmware?
> >>>
> >>> I was thinking 'hciattach' but I was having trouble.  I was hoping you
> >>> might have some insight.
> >>>
> >>>  hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow  Just
> >>> returns a timeout.
> >>>
> >>> I modified my i.MX6 device tree per the binding documentation and
> >>> setup the regulators and enable GPIO pins.
> >>
> >> If you configured everything correctly no userspace interaction is
> >> required. The driver should request the firmware automatically once
> >> you power up the bluetooth device.
> >>
> >> Apart from DT changes make sure, that the following options are
> >> enabled and check dmesg for any hints.
> >>
> >> CONFIG_SERIAL_DEV_BUS
> >> CONFIG_SERIAL_DEV_CTRL_TTYPORT
> >> CONFIG_BT_HCIUART
> >> CONFIG_BT_HCIUART_LL
> >
> > I have enabled those flags, and I have updated my device tree.
> > I am testing this on an OMAP3630 (DM3730) board with a WL1283.  I am
> > getting a lot of timeout errors.  I tested this against the original
> > implemention I had in pdata-quirks.c using the ti-st driver, uim & and
> > the btwilink driver.
> >
> > I pulled in some of the newer patches to enable the wl1283-st, but I
> > am obviously missing something.
> >
> > I   58.717651] Bluetooth: hci0: Reading TI version information failed
> > (-110)
> > [   58.724853] Bluetooth: hci0: download firmware failed, retrying...
> > [   60.957641] Bluetooth: hci0 command 0x1001 tx timeout
> > [   68.957641] Bluetooth: hci0: Reading TI version information failed
> > (-110)
> > [   68.964843] Bluetooth: hci0: download firmware failed, retrying...
> > [   69.132171] Bluetooth: Unknown HCI packet type 06
> > [   69.138244] Bluetooth: Unknown HCI packet type 0c
> > [   69.143249] Bluetooth: Unknown HCI packet type 40
> > [   69.148498] Bluetooth: Unknown HCI packet type 20
> > [   69.153533] Bluetooth: Data length is too large
> > [   69.158569] Bluetooth: Unknown HCI packet type a0
> > [   69.163574] Bluetooth: Unknown HCI packet type 00
> > [   69.168731] Bluetooth: Unknown HCI packet type 00
> > [   69.173736] Bluetooth: Unknown HCI packet type 34
> > [   69.178924] Bluetooth: Unknown HCI packet type 91
> > [   71.197631] Bluetooth: hci0 command 0x1001 tx timeout
> > [   79.197662] Bluetooth: hci0: Reading TI version information failed (-110)
> 
> There's a bug in serdev_device_write(), so if you have that function
> you need either the fix I sent or the patch to make
> serdev_device_writebuf atomic again. Both are on the linux-serial
> list, but not in any tree yet.

You refer to the patches below, right?

  [PATCH] tty: serdev: fix serdev_device_write return value, 
  http://www.spinics.net/lists/linux-serial/msg26117.html

  [PATCH] serdev: Restore serdev_device_write_buf for atomic context,
  http://www.spinics.net/lists/linux-serial/msg26113.html

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 RFC net-next 3/6] net: Introduce IFF_LWT_NETDEV flag
From: Roopa Prabhu @ 2017-05-09  5:04 UTC (permalink / raw)
  To: David Ahern
  Cc: David Miller, Johannes Berg, netdev@vger.kernel.org,
	Florian Fainelli, Nicolas Dichtel
In-Reply-To: <52621c1c-57c4-efb7-80dc-648a0da7ae7f@gmail.com>

On Mon, May 8, 2017 at 5:57 PM, David Ahern <dsahern@gmail.com> wrote:
> On 5/8/17 1:11 PM, David Miller wrote:
>> From: Johannes Berg <johannes@sipsolutions.net>
>> Date: Mon, 08 May 2017 10:55:12 +0200
>>
>>>
>>>> +static inline bool netif_is_lwd(struct net_device *dev)
>>>> +{
>>>> +   return !!(dev->priv_flags & IFF_LWT_NETDEV);
>>>> +}
>>>
>>> Am I the only one who thinks that this "LWT_NETDEV" vs "LWD" is a bit
>>> confusing?
>>
>> Agreed, my old eyes can't discern them at a distance :-)
>>
>
> perhaps it is the tiny font your old eyes are having trouble with :-)
>
> I am fine with Johannes' suggestion -- just spell it out:
>     netif_is_lwt_netdev
>
> where lwt = LightWeighT

makes sense...but this does sound like a 'light weight tunnel
netdevice' though.....just cause 'LWT' already expands to 'light
weight tunnel'

^ permalink raw reply

* Re: [PATCH] net: wireless: ath: ath10k: remove unnecessary code
From: Kalle Valo @ 2017-05-09  5:33 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: netdev@vger.kernel.org, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <20170509042159.GA19727@embeddedgus>

"Gustavo A. R. Silva" <garsilva@embeddedor.com> writes:

> The name of an array used by itself will always return the array's address.
> So these tests will always evaluate as false and therefore the _return_
> will never be executed.
>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

I don't understand the commit log, especially what does "The name of an
array used by itself" mean?

-- 
Kalle Valo

^ permalink raw reply

* [PATCH] net: fec: select queue depending on VLAN priority
From: Stefan Agner @ 2017-05-09  5:37 UTC (permalink / raw)
  To: fugang.duan; +Cc: andrew, festevam, netdev, linux-kernel, Stefan Agner

Since the addition of the multi queue code with commit 59d0f7465644
("net: fec: init multi queue date structure") the queue selection
has been handelt by the default transmit queue selection
implementation which tries to evenly distribute the traffic across
all available queues. This selection presumes that the queues are
using an equal priority, however, the queues 1 and 2 are actually
of higher priority (the classification of the queues is enabled in
fec_enet_enable_ring).

This can lead to net scheduler warnings and continuous TX ring
dumps when exercising the system with iperf.

Use only queue 0 for all common traffic (no VLAN and P802.1p
priority 0 and 1) and route level 2-7 through queue 1 and 2.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
---
This patch solves the issue documented in this thread:
https://www.spinics.net/lists/netdev/msg430679.html

 drivers/net/ethernet/freescale/fec_main.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 91a16641e851..72343cbfddc9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -72,6 +72,7 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
 #define DRIVER_NAME	"fec"
 
 #define FEC_ENET_GET_QUQUE(_x) ((_x == 0) ? 1 : ((_x == 1) ? 2 : 0))
+static const u16 fec_enet_vlan_pri_to_queue[8] = {0, 0, 1, 1, 1, 2, 2, 2};
 
 /* Pause frame feild and FIFO threshold */
 #define FEC_ENET_FCE	(1 << 5)
@@ -3052,10 +3053,28 @@ static int fec_set_features(struct net_device *netdev,
 	return 0;
 }
 
+u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
+			  void *accel_priv, select_queue_fallback_t fallback)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	u16 vlan_tci, vlan_pcp;
+
+	if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
+		return fallback(ndev, skb);
+
+	/* Assign queue 0 if no VLAN tag present */
+	if (vlan_get_tag(skb, &vlan_tci) < 0)
+		return 0;
+
+	vlan_pcp = (vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+	return fec_enet_vlan_pri_to_queue[vlan_pcp];
+}
+
 static const struct net_device_ops fec_netdev_ops = {
 	.ndo_open		= fec_enet_open,
 	.ndo_stop		= fec_enet_close,
 	.ndo_start_xmit		= fec_enet_start_xmit,
+	.ndo_select_queue       = fec_enet_select_queue,
 	.ndo_set_rx_mode	= set_multicast_list,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_tx_timeout		= fec_timeout,
-- 
2.12.2

^ permalink raw reply related

* Re: sky2: Use seq_putc() in sky2_debug_show()
From: Stephen Hemminger @ 2017-05-09  5:50 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Lino Sanfilippo, netdev, Mirko Lindner, LKML, kernel-janitors
In-Reply-To: <a85959a8-a357-d8e0-d7e9-617e2052aed6@users.sourceforge.net>

On Mon, 8 May 2017 19:42:46 +0200
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> > Which issue do you mean? I dont see any issue you fix here.  
> 
> Are the run time characteristics a bit nicer for the function “seq_putc”
> in comparison to the function “seq_puts” for printing a single line break here?
> 
> Regards,
> Markus

I would put this in why bother category. seq_puts is correct and this is only
in diagnostic output useful to developer and disabled on most distro kernels

^ permalink raw reply

* [PATCH] ip: mpls: fix printing of mpls labels
From: David Ahern @ 2017-05-09  6:04 UTC (permalink / raw)
  To: stephen, netdev; +Cc: roopa, David Ahern

If the kernel returns more labels than iproute2 expects, none of
the labels are printed and (null) is shown instead:
    $ ip -f mpls ro ls
    101 as to (null) via inet 172.16.2.2 dev virt12
    201 as to 202/203 via inet6 2001:db8:2::2 dev virt12

Remove the use of MPLS_MAX_LABELS and rely on buffer length that is
passed to mpls_ntop. With this change ip can print the label stack
returned by the kernel up to 255 characters (limit is due to size of
buf passed in) which amounts to 31 labels with a separator.

With this change the above is:
    $ ip/ip -f mpls ro ls
    101 as to 102/103/104/105/106/107/108/109/110 via inet 172.16.2.2 dev virt12

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 lib/mpls_ntop.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/mpls_ntop.c b/lib/mpls_ntop.c
index 945d6d5e..5902f503 100644
--- a/lib/mpls_ntop.c
+++ b/lib/mpls_ntop.c
@@ -10,18 +10,20 @@ static const char *mpls_ntop1(const struct mpls_label *addr, char *buf, size_t b
 {
 	size_t destlen = buflen;
 	char *dest = buf;
-	int count;
+	int count = 0;
 
-	for (count = 0; count < MPLS_MAX_LABELS; count++) {
-		uint32_t entry = ntohl(addr[count].entry);
+	while (1) {
+		uint32_t entry = ntohl(addr[count++].entry);
 		uint32_t label = (entry & MPLS_LS_LABEL_MASK) >> MPLS_LS_LABEL_SHIFT;
 		int len = snprintf(dest, destlen, "%u", label);
 
+		if (len >= destlen)
+			break;
+
 		/* Is this the end? */
 		if (entry & MPLS_LS_S_MASK)
 			return buf;
 
-
 		dest += len;
 		destlen -= len;
 		if (destlen) {
-- 
2.11.0 (Apple Git-81)

^ permalink raw reply related

* Re: [PATCH iproute2] tc: bpf: add ppc64 and sparc64 to list of archs with eBPF support
From: Stephen Hemminger @ 2017-05-09  6:06 UTC (permalink / raw)
  To: Alexander Alemayhu; +Cc: netdev, daniel
In-Reply-To: <1494102610-19853-1-git-send-email-alexander@alemayhu.com>

On Sat,  6 May 2017 22:30:10 +0200
Alexander Alemayhu <alexander@alemayhu.com> wrote:

> sparc64 support was added in 7a12b5031c6b (sparc64: Add eBPF JIT., 2017-04-17)[0]
> and ppc64 in 156d0e290e96 (powerpc/ebpf/jit: Implement JIT compiler for extended BPF, 2016-06-22)[1].
> 
> [0]: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=7a12b5031c6b
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=156d0e290e96
> Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
> ---
>  man/man8/tc-bpf.8 | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
> index e371964d06ab..2e9812ede028 100644
> --- a/man/man8/tc-bpf.8
> +++ b/man/man8/tc-bpf.8
> @@ -75,9 +75,9 @@ In Linux, it's generally considered that eBPF is the successor of cBPF.
>  The kernel internally transforms cBPF expressions into eBPF expressions and
>  executes the latter. Execution of them can be performed in an interpreter
>  or at setup time, they can be just-in-time compiled (JIT'ed) to run as
> -native machine code. Currently, x86_64, ARM64 and s390 architectures have
> -eBPF JIT support, whereas PPC, SPARC, ARM and MIPS have cBPF, but did not
> -(yet) switch to eBPF JIT support.
> +native machine code. Currently, x86_64, ARM64, s390, ppc64 and sparc64
> +architectures have eBPF JIT support, whereas PPC, SPARC, ARM and MIPS have
> +cBPF, but did not (yet) switch to eBPF JIT support.
>  
>  eBPF's instruction set has similar underlying principles as the cBPF
>  instruction set, it however is modelled closer to the underlying

Applied thanks.

^ permalink raw reply

* Re: [PATCH] wcn36xx: Close SMD channel on device removal
From: Kalle Valo @ 2017-05-09  6:17 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Eugene Krasnikov, Eyal Ilsar, wcn36xx, linux-wireless, netdev,
	linux-kernel, linux-arm-msm
In-Reply-To: <20170509043637.28179-1-bjorn.andersson@linaro.org>

Bjorn Andersson <bjorn.andersson@linaro.org> writes:

> The SMD channel is not the primary WCNSS channel and must explicitly be
> closed as the device is removed, or the channel will already by open on
> a subsequent probe call in e.g. the case of reloading the kernel module.
>
> This issue was introduced because I simplified the underlying SMD
> implementation while the SMD adaptions of the driver sat on the mailing
> list, but missed to update these patches. The patch does however only
> apply back to the transition to rpmsg, hence the limited Fixes.
>
> Fixes: 5052de8deff5 ("soc: qcom: smd: Transition client drivers from smd to rpmsg")
> Reported-by: Eyal Ilsar <c_eilsar@qti.qualcomm.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

As this is a regression I'll queue this to 4.12.

But if this is an older bug (didn't quite understand your description
though) should there be a separate patch for stable releases?

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate
From: Alexandru Ardelean @ 2017-05-09  6:56 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20170508.153414.1871607886646871422.davem@davemloft.net>

On Mon, May 8, 2017 at 10:34 PM, David Miller <davem@davemloft.net> wrote:
> From: Alexandru Ardelean <ardeleanalex@gmail.com>
> Date: Mon,  8 May 2017 14:31:18 +0300
>
>> Looking at xgmac_wait_until_done() and xgmac_wait_until_free()
>> functions, they seem to have turned out completely identical.
>>
>> Though, judging from the git history it seems they
>> initially weren't.
>>
>> Remove xgmac_wait_until_free() in favor of xgmac_wait_until_done().
>>
>> Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
>
> This situation got created by the commit below, I wonder if it
> is even correct.
>
> Certainly the original author of this code intended the MDIO
> _data_ rather than the _status_ register to be polled.
>
> Otherwise, why in the world have two functions which are in
> every other regard identical?

[ Removed Shaohui.Xie@nxp.com email from thread ; seems email addr no
longer valid ].

I should have added an RFC tag.
Thing is, we just use this driver, so, I can't offer much input here.

I was debugging [a while ago] some other issue related to this and
stumbled over this.
I haven't seem any obvious issues yet, to suggest one thing or or the
other [regarding polling status or data registers].

I would have liked some input from some NXP/Freescale people on this email list.
Or maybe I could forward this to more people that contribute the NXP eco-system.

If you have any suggestions, please advise and I'll adapt.

Thanks
Alex

>
> commit 26eee0210ad72a29eb4a70b34320bda266f91a0d
> Author: Shaohui Xie <Shaohui.Xie@freescale.com>
> Date:   Mon Mar 16 18:55:50 2015 +0800
>
>     net/fsl: fix a bug in xgmac_mdio
>
>     There is a bug in xgmac_wait_until_done() which mdio_stat should be used
>     instead of mdio_data when checking if busy bit is cleared.
>
>     Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> index 3a83bc2..5f691f2 100644
> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> @@ -79,7 +79,7 @@ static int xgmac_wait_until_done(struct device *dev,
>
>         /* Wait till the MDIO write is complete */
>         timeout = TIMEOUT;
> -       while ((ioread32be(&regs->mdio_data) & MDIO_DATA_BSY) && timeout) {
> +       while ((ioread32be(&regs->mdio_stat) & MDIO_STAT_BSY) && timeout) {
>                 cpu_relax();
>                 timeout--;
>         }

^ permalink raw reply

* Re: [RFC iproute2 0/8] RDMA tool
From: Leon Romanovsky @ 2017-05-09  7:03 UTC (permalink / raw)
  To: Dennis Dalessandro
  Cc: Bart Van Assche, jiri-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ram.amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
	sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	hch-jcswGhMUV9g@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	ariela-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <b82f5d3d-f198-3410-af85-85befc14a2ec-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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

On Mon, May 08, 2017 at 11:55:25AM -0400, Dennis Dalessandro wrote:
> On 05/04/2017 03:42 PM, Leon Romanovsky wrote:
> > On Thu, May 04, 2017 at 03:26:13PM -0400, Dennis Dalessandro wrote:
> > > On 05/04/2017 02:45 PM, Leon Romanovsky wrote:
> > > > On Thu, May 04, 2017 at 06:30:27PM +0000, Bart Van Assche wrote:
> > > > > On Thu, 2017-05-04 at 21:25 +0300, Leon Romanovsky wrote:
> > > > > > On Thu, May 04, 2017 at 06:10:54PM +0000, Bart Van Assche wrote:
> > > > > > > On Thu, 2017-05-04 at 21:02 +0300, Leon Romanovsky wrote:
> > > > > > > > Following our discussion both in mailing list [1] and at the LPC 2016 [2],
> > > > > > > > we would like to propose this RDMA tool to be part of iproute2 package
> > > > > > > > and finally improve this situation.
> > > > > > >
> > > > > > > Hello Leon,
> > > > > > >
> > > > > > > Although I really appreciate your work: can you clarify why you would like to
> > > > > > > add *RDMA* functionality to an *IP routing* tool? I haven't found any motivation
> > > > > > > for adding RDMA functionality to iproute2 in [1].
> > > > > >
> > > > > > We are planning to reuse the same infrastructure provided by iproute2,
> > > > > > like netlink parsing, access to distributions, same CLI and same standards.
> > > > > >
> > > > > > Right now, RDMA is already tightened to netdev: iWARP, RoCE, IPoIB, HFI-VNIC.
> > > > > > Many drivers (mlx, qed, i40, cxgb) are sharing code between net and
> > > > > > RDMA.
> > > > > >
> > > > > > I do expect that iproute2 will be installed on every machine with any
> > > > > > type of connection, including IB and OPA.
> > > > > >
> > > > > > So I think that it is enough to be part of that suite and don't invent
> > > > > > our own for one specific tool.
> > > > >
> > > > > Hello Leon,
> > > > >
> > > > > Sorry but to me that sounds like a weak argument for including RDMA functionality
> > > > > in iproute2. There is already a library for communication over netlink sockets,
> > > > > namely libnl. Is there functionality that is in iproute2 but not in libnl and
> > > > > that is needed for the new tool? If so, have you considered to create a new
> > > > > library for that functionality?
> > > >
> > > > It is not hard to create new tool, the hardest part is to ensure that it is
> > > > part of the distributions. Did you count how many months we are trying to
> > > > add rdma-core to debian?
> > >
> > > I do agree that it is a strange pairing and am not really a fan. However at
> > > the end of the day it's just a name for a repo/package. If the iproute folks
> > > are fine to include rdma in their repo/package, great we can leverage their
> > > code for CLI and other common stuff.
> > >
> > > Now if the interface was something like "ip -FlagForRdma ..." I would object
> > > to that, but the interface is "rdma ... " so from users perspective it's
> > > different tools. They don't need to care that it was sourced from a common
> > > git repo.
> > >
> > > Just as an aside this already works a bit with OPA:
> > >
> > >  $ ./rdma link
> > > 1/1: hfi1_0/1: ifname NONE cap_mask 0x00410022 lid 0x1 lid_mask_count 0
> > > link_layer InfiniBand
> > >          phys_state 5: LinkUp rate 100 Gb/sec (4X EDR) sm_lid 0x1 sm_sl 0
> > > state 4: ACTIVE
> > >
> > > Leon I'll get you more feedback and testing, I've just been really bogged
> > > down this week, sorry.
> >
> > Thanks Denny,
> >
> > Before you are starting to test it, can you please provide your feedback
> > on my initial questions? Usability and need of sysfs.
> > ----
> > This is initial phase to understand if user experience for this tool fits
> > RDMA and netdev communities exepectations. Also I would like to get feedback
> > if it is really worth to provide legacy sysfs for old kernels, or maybe I should
> > implement netlink from the beginning and abandon sysfs completely.
> > -----
>
> For the initial phase I think you did the right thing by reading sysfs. I
> like the ability for the tool to be compatible with legacy kernels, but at
> the same time I don't know if it's worth the hassle. I won't fight too hard
> either way.
>
> Perhaps we should take a stab and seeing what a dual sysfs/netlink interface
> would look like, and see just how hard and complicated it really is. Then we
> can make that call. You already have the sysfs version, and have to do
> netink anyway, so let's not rip out what's there just yet, this is an RFC
> after all, not like you are asking for this exact version to be merged yet.

Not at all, it is too early to merge it, at the end it is POC.

>
> As far as usability, I think what's here is a great start and we can
> continue to refine. I'm particularly interested in the stats capabilities.
> In particular being able to filter what stats are shown and watch as they
> change over time. RDMA devices have lots of counters and stats. A common
> tool for users to be able to get that data across HW types would be a really
> good thing.

Yeah, the statistics will be tough nut to crack.


>
> -Denny
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] qca_debug: Reduce function calls for sequence output in qcaspi_info_show()
From: Stefan Wahren @ 2017-05-09  7:07 UTC (permalink / raw)
  To: SF Markus Elfring, netdev, David S. Miller, Philippe Reynes
  Cc: LKML, kernel-janitors
In-Reply-To: <9932ee63-f766-49ce-d760-9a0bd06f0657@users.sourceforge.net>

Am 08.05.2017 um 19:29 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 8 May 2017 19:21:27 +0200
>
> A bit of data was put into a sequence by separate function calls.
> Print the same data together with adjusted seq_printf() calls instead.

Sorry, i'm not happy with this patch. It doesn't increase readabilityand
mixes the output of multiple lines.

The only benefit is a little bit higher performance. But for debugfs
this won't be necessary.

Stefan

>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/net/ethernet/qualcomm/qca_debug.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>

^ permalink raw reply

* [PATCH 0/3] ath9k: Fine-tuning for some function implementations
From: SF Markus Elfring @ 2017-05-09  7:25 UTC (permalink / raw)
  To: ath9k-devel, linux-wireless, netdev, Kalle Valo; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 09:19:09 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Reduce function calls for sequence output in read_file_dma()
  Replace four seq_printf() calls by seq_putc()
  Adjust a null pointer check in three functions

 drivers/net/wireless/ath/ath9k/debug.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

-- 
2.12.2

^ permalink raw reply

* [PATCH 1/3] ath9k: Reduce function calls for sequence output in read_file_dma()
From: SF Markus Elfring @ 2017-05-09  7:26 UTC (permalink / raw)
  To: ath9k-devel, linux-wireless, netdev, Kalle Valo; +Cc: LKML, kernel-janitors
In-Reply-To: <03575993-754d-924c-9736-2a8d19e98fa5@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 May 2017 21:55:09 +0200

Some data were put into a sequence by separate function calls.
Print the same data with two function calls less.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ath/ath9k/debug.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 2e64977a8ab6..981b38a1e352 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -427,9 +427,7 @@ static int read_file_dma(struct seq_file *file, void *data)
 		seq_printf(file, "%d: %08x ", i, val[i]);
 	}
 
-	seq_puts(file, "\n\n");
-	seq_puts(file, "Num QCU: chain_st fsp_ok fsp_st DCU: chain_st\n");
-
+	seq_puts(file, "\n\nNum QCU: chain_st fsp_ok fsp_st DCU: chain_st\n");
 	for (i = 0; i < ATH9K_NUM_QUEUES; i++, qcuOffset += 4, dcuOffset += 5) {
 		if (i == 8) {
 			qcuOffset = 0;
@@ -448,9 +446,8 @@ static int read_file_dma(struct seq_file *file, void *data)
 			   (*dcuBase & (0x1f << dcuOffset)) >> dcuOffset);
 	}
 
-	seq_puts(file, "\n");
-
-	seq_printf(file, "qcu_stitch state:   %2x    qcu_fetch state:        %2x\n",
+	seq_printf(file,
+		   "\nqcu_stitch state:   %2x    qcu_fetch state:        %2x\n",
 		   (val[3] & 0x003c0000) >> 18, (val[3] & 0x03c00000) >> 22);
 	seq_printf(file, "qcu_complete state: %2x    dcu_complete state:     %2x\n",
 		   (val[3] & 0x1c000000) >> 26, (val[6] & 0x3));
-- 
2.12.2

^ permalink raw reply related

* [PATCH 2/3] ath9k: Replace four seq_printf() calls by seq_putc()
From: SF Markus Elfring @ 2017-05-09  7:28 UTC (permalink / raw)
  To: ath9k-devel, linux-wireless, netdev, Kalle Valo; +Cc: LKML, kernel-janitors
In-Reply-To: <03575993-754d-924c-9736-2a8d19e98fa5@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 May 2017 22:04:47 +0200

Four single characters (line breaks) should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ath/ath9k/debug.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 981b38a1e352..0d215598193c 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -421,7 +421,7 @@ static int read_file_dma(struct seq_file *file, void *data)
 
 	for (i = 0; i < ATH9K_NUM_DMA_DEBUG_REGS; i++) {
 		if (i % 4 == 0)
-			seq_puts(file, "\n");
+			seq_putc(file, '\n');
 
 		val[i] = REG_READ_D(ah, AR_DMADBG_0 + (i * sizeof(u32)));
 		seq_printf(file, "%d: %08x ", i, val[i]);
@@ -702,8 +702,7 @@ static int read_file_misc(struct seq_file *file, void *data)
 	if (rxfilter & ATH9K_RX_FILTER_CONTROL_WRAPPER)
 		seq_puts(file, " CONTROL_WRAPPER");
 
-	seq_puts(file, "\n");
-
+	seq_putc(file, '\n');
 	reg = sc->sc_ah->imask;
 
 	seq_printf(file, "INTERRUPT-MASK: 0x%x", reg);
@@ -723,8 +722,7 @@ static int read_file_misc(struct seq_file *file, void *data)
 	if (reg & ATH9K_INT_BB_WATCHDOG)
 		seq_puts(file, " BB_WATCHDOG");
 
-	seq_puts(file, "\n");
-
+	seq_putc(file, '\n');
 	i = 0;
 	ath_for_each_chanctx(sc, ctx) {
 		if (list_empty(&ctx->vifs))
@@ -981,7 +979,7 @@ static int read_file_dump_nfcal(struct seq_file *file, void *data)
 		seq_printf(file, " %d\t %d\t %d\t\t", i, h[i].privNF, nread);
 		for (j = 0; j < nread; j++)
 			seq_printf(file, " %d", h[i].nfCalBuffer[j]);
-		seq_puts(file, "\n");
+		seq_putc(file, '\n');
 	}
 
 	return 0;
-- 
2.12.2

^ permalink raw reply related

* [PATCH 3/3] ath9k: Adjust a null pointer check in three functions
From: SF Markus Elfring @ 2017-05-09  7:29 UTC (permalink / raw)
  To: ath9k-devel, linux-wireless, netdev, Kalle Valo; +Cc: LKML, kernel-janitors
In-Reply-To: <03575993-754d-924c-9736-2a8d19e98fa5@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 May 2017 22:17:13 +0200

The script "checkpatch.pl" pointed information out like the following.

Comparison to NULL could be written "!buf"

Thus adjust this expression.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ath/ath9k/debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 0d215598193c..5f2c1d8e8e70 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -161,7 +161,7 @@ static ssize_t read_file_ani(struct file *file, char __user *user_buf,
 	};
 
 	buf = kzalloc(size, GFP_KERNEL);
-	if (buf == NULL)
+	if (!buf)
 		return -ENOMEM;
 
 	len += scnprintf(buf + len, size - len, "%15s: %s\n", "ANI",
@@ -315,7 +315,7 @@ static ssize_t read_file_antenna_diversity(struct file *file,
 	};
 
 	buf = kzalloc(size, GFP_KERNEL);
-	if (buf == NULL)
+	if (!buf)
 		return -ENOMEM;
 
 	if (!(pCap->hw_caps & ATH9K_HW_CAP_ANT_DIV_COMB)) {
@@ -1008,7 +1008,7 @@ static ssize_t read_file_btcoex(struct file *file, char __user *user_buf,
 	size_t retval;
 
 	buf = kzalloc(size, GFP_KERNEL);
-	if (buf == NULL)
+	if (!buf)
 		return -ENOMEM;
 
 	if (!sc->sc_ah->common.btcoex_enabled) {
-- 
2.12.2


^ permalink raw reply related

* DQL and TCQ_F_CAN_BYPASS destroy performance under virtualizaiton (Was: "Re: net_sched strange in 4.11")
From: Anton Ivanov @ 2017-05-09  7:46 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Stefan Hajnoczi
In-Reply-To: <66c5c2d3-65df-8506-6934-5628e709e229@cambridgegreys.com>

I have figured it out. Two issues.

1) skb->xmit_more is hardly ever set under virtualization because the 
qdisc is usually bypassed because of TCQ_F_CAN_BYPASS. Once 
TCQ_F_CAN_BYPASS is set a virtual NIC driver is not likely see 
skb->xmit_more (this answers my "how does this work at all" question).

2) If that flag is turned off (I patched sched_generic to turn it off in 
pfifo_fast while testing), DQL keeps xmit_more from being set. If the 
driver is not DQL enabled xmit_more is never ever set. If the driver is 
DQL enabled the queue is adjusted to ensure xmit_more stops happening 
within 10-15 xmit cycles.

That is plain *wrong* for virtual NICs - virtio, emulated NICs, etc. 
There, the BIG cost is telling the hypervisor that it needs to "kick" 
the packets. The cost of putting them into the vNIC buffers is 
negligible. You want xmit_more to happen - it makes between 50% and 300% 
(depending on vNIC design) difference. If there is no xmit_more the vNIC 
will immediately "kick" the hypervisor and try to signal that  the 
packet needs to move straight away (as for example in virtio_net).

In addition to that, the perceived line rate is proportional to this 
cost, so I am not sure that the current dql math holds. In fact, I think 
it does not - it is trying to adjust something which influences the 
perceived line rate.

So - how do we turn BOTH bypass and DQL adjustment while under 
virtualization and set them to be "always qdisc" + "always xmit_more 
allowed"

A.

P.S. Cc-ing virtio maintainer

A.


On 08/05/17 08:15, Anton Ivanov wrote:
> Hi all,
>
> I was revising some of my old work for UML to prepare it for 
> submission and I noticed that skb->xmit_more does not seem to be set 
> any more.
>
> I traced the issue as far as net/sched/sched_generic.c
>
> try_bulk_dequeue_skb() is never invoked (the drivers I am working on 
> are dql enabled so that is not the problem).
>
> More interestingly, if I put a breakpoint and debug output into 
> dequeue_skb() around line 147 - right before the bulk: tag that skb 
> there is always NULL. ???
>
> Similarly, debug in pfifo_fast_dequeue shows only NULLs being 
> dequeued. Again - ???
>
> First and foremost, I apologize for the silly question, but how can 
> this work at all? I see the skbs showing up at the driver level, why 
> are NULLs being returned at qdisc dequeue and where do the skbs at the 
> driver level come from?
>
> Second, where should I look to fix it?
>
> A.
>


-- 
Anton R. Ivanov

Cambridge Greys Limited, England company No 10273661
http://www.cambridgegreys.com/

^ permalink raw reply

* [PATCH] wil6210: Replace five seq_puts() calls by seq_putc()
From: SF Markus Elfring @ 2017-05-09  7:50 UTC (permalink / raw)
  To: wil6210, linux-wireless, netdev, Kalle Valo, Maya Erez
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 May 2017 22:22:04 +0200

Five single characters (line breaks) should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ath/wil6210/debugfs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 5648ebbd0e16..90118d286fb9 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -76,11 +76,11 @@ static void wil_print_vring(struct seq_file *s, struct wil6210_priv *wil,
 			volatile struct vring_tx_desc *d = &vring->va[i].tx;
 
 			if ((i % 128) == 0 && (i != 0))
-				seq_puts(s, "\n");
+				seq_putc(s, '\n');
 			seq_printf(s, "%c", (d->dma.status & BIT(0)) ?
 					_s : (vring->ctx[i].skb ? _h : 'h'));
 		}
-		seq_puts(s, "\n");
+		seq_putc(s, '\n');
 	}
 	seq_puts(s, "}\n");
 }
@@ -233,7 +233,7 @@ static void wil_print_ring(struct seq_file *s, const char *prefix,
 				wil_seq_hexdump(s, databuf, len, "      : ");
 			}
 		} else {
-			seq_puts(s, "\n");
+			seq_putc(s, '\n');
 		}
 	}
  out:
@@ -1366,7 +1366,7 @@ static void wil_print_rxtid_crypto(struct seq_file *s, int tid,
 		seq_printf(s, " [%i%s]%6phN", i, cc->key_set ? "+" : "-",
 			   cc->pn);
 	}
-	seq_puts(s, "\n");
+	seq_putc(s, '\n');
 }
 
 static int wil_sta_debugfs_show(struct seq_file *s, void *data)
@@ -1423,7 +1423,7 @@ __acquires(&p->tid_rx_lock) __releases(&p->tid_rx_lock)
 			     mcs++)
 				seq_printf(s, " %lld",
 					   p->stats.rx_per_mcs[mcs]);
-			seq_puts(s, "\n");
+			seq_putc(s, '\n');
 		}
 	}
 
-- 
2.12.2

^ permalink raw reply related

* Re: [PATCH] wil6210: Replace five seq_puts() calls by seq_putc()
From: Johannes Berg @ 2017-05-09  7:53 UTC (permalink / raw)
  To: SF Markus Elfring, wil6210, linux-wireless, netdev, Kalle Valo,
	Maya Erez
  Cc: LKML, kernel-janitors
In-Reply-To: <64747f85-e373-a0ff-b6dc-70cdfe35f71a@users.sourceforge.net>

On Tue, 2017-05-09 at 09:50 +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 8 May 2017 22:22:04 +0200
> 
> Five single characters (line breaks) should be put into a sequence.
> Thus use the corresponding function "seq_putc".

Please stop, this isn't really an issue worth worrying about.

johannes

^ 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