Netdev List
 help / color / mirror / Atom feed
* [PATCH 00/35] constify net usb_device_id
From: Arvind Yadav @ 2017-08-08 15:55 UTC (permalink / raw)
  To: kvalo, wg, mkl, linux-can, netdev; +Cc: linux-kernel

usb_device_id are not supposed to change at runtime. All functions
working with usb_device_id provided by <linux/usb.h> work with
const usb_device_id. So mark the non-const structs as const.

[PATCH 10/35] :Fix checkpatch.pl error.
ERROR: space prohibited before open square bracket '['.

Arvind Yadav (35):
  [PATCH 01/35] can: ems_usb: constify usb_device_id
  [PATCH 02/35] can: esd_usb2: constify usb_device_id
  [PATCH 03/35] can: peak_usb: constify usb_device_id
  [PATCH 04/35] net: irda: irda-usb: constify usb_device_id
  [PATCH 05/35] net: irda: kingsun: constify usb_device_id
  [PATCH 06/35] net: irda: ks959: constify usb_device_id
  [PATCH 07/35] net: irda: ksdazzle: constify usb_device_id
  [PATCH 08/35] net: irda: mcs7780: constify usb_device_id
  [PATCH 09/35] net: irda: stir4200: constify usb_device_id
  [PATCH 10/35] net: usb: catc: constify usb_device_id and and fix space before '[' error
  [PATCH 11/35] net: usb: cdc-phonet: constify usb_device_id
  [PATCH 12/35] net: usb: ipheth: constify usb_device_id
  [PATCH 13/35] net: usb: kaweth: constify usb_device_id
  [PATCH 14/35] net: usb: r8152: constify usb_device_id
  [PATCH 15/35] net: usb: rtl8150: constify usb_device_id
  [PATCH 16/35] wireless: ath: ar5523: constify usb_device_id
  [PATCH 17/35] wireless: ath: ath6kl: constify usb_device_id
  [PATCH 18/35] wireless: ath: ath9k: constify usb_device_id
  [PATCH 19/35] wireless: ath: carl9170: constify usb_device_id
  [PATCH 20/35] wireless: atmel: at76c50x: constify usb_device_id
  [PATCH 21/35] wireless: broadcom: brcm80211: constify usb_device_id
  [PATCH 22/35] wireless: intersil: orinoco: constify usb_device_id
  [PATCH 23/35] wireless: intersil: p54: constify usb_device_id
  [PATCH 24/35] wireless: marvell: libertas: constify usb_device_id
  [PATCH 25/35] wireless: marvell: libertas_tf: constify usb_device_id
  [PATCH 26/35] wireless: marvell: mwifiex: constify usb_device_id
  [PATCH 27/35] wireless: mediatek: mt7601u: constify usb_device_id
  [PATCH 28/35] wireless: ralink: rt2500usb: constify usb_device_id
  [PATCH 29/35] wireless: ralink: rt2800usb: constify usb_device_id
  [PATCH 30/35] wireless: ralink: rt73usb: constify usb_device_id
  [PATCH 31/35] wireless: realtek: rtl8187: constify usb_device_id
  [PATCH 32/35] wireless: realtek: rtl8xxxu: constify usb_device_id
  [PATCH 33/35] wireless: realtek: rtl8192cu: constify usb_device_id
  [PATCH 34/35] wireless: zydas: zd1201: constify usb_device_id
  [PATCH 35/35] wireless: zydas: zd1211rw: constify usb_device_id

 drivers/net/can/usb/ems_usb.c                          | 2 +-
 drivers/net/can/usb/esd_usb2.c                         | 2 +-
 drivers/net/can/usb/peak_usb/pcan_usb_core.c           | 2 +-
 drivers/net/irda/irda-usb.c                            | 2 +-
 drivers/net/irda/kingsun-sir.c                         | 2 +-
 drivers/net/irda/ks959-sir.c                           | 2 +-
 drivers/net/irda/ksdazzle-sir.c                        | 2 +-
 drivers/net/irda/mcs7780.c                             | 2 +-
 drivers/net/irda/stir4200.c                            | 2 +-
 drivers/net/usb/catc.c                                 | 2 +-
 drivers/net/usb/cdc-phonet.c                           | 2 +-
 drivers/net/usb/ipheth.c                               | 2 +-
 drivers/net/usb/kaweth.c                               | 2 +-
 drivers/net/usb/r8152.c                                | 2 +-
 drivers/net/usb/rtl8150.c                              | 2 +-
 drivers/net/wireless/ath/ar5523/ar5523.c               | 2 +-
 drivers/net/wireless/ath/ath6kl/usb.c                  | 2 +-
 drivers/net/wireless/ath/ath9k/hif_usb.c               | 2 +-
 drivers/net/wireless/ath/carl9170/usb.c                | 2 +-
 drivers/net/wireless/atmel/at76c50x-usb.c              | 2 +-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 +-
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c    | 2 +-
 drivers/net/wireless/intersil/p54/p54usb.c             | 2 +-
 drivers/net/wireless/marvell/libertas/if_usb.c         | 2 +-
 drivers/net/wireless/marvell/libertas_tf/if_usb.c      | 2 +-
 drivers/net/wireless/marvell/mwifiex/usb.c             | 2 +-
 drivers/net/wireless/mediatek/mt7601u/usb.c            | 2 +-
 drivers/net/wireless/ralink/rt2x00/rt2500usb.c         | 2 +-
 drivers/net/wireless/ralink/rt2x00/rt2800usb.c         | 2 +-
 drivers/net/wireless/ralink/rt2x00/rt73usb.c           | 2 +-
 drivers/net/wireless/realtek/rtl818x/rtl8187/dev.c     | 2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 2 +-
 drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c    | 2 +-
 drivers/net/wireless/zydas/zd1201.c                    | 2 +-
 drivers/net/wireless/zydas/zd1211rw/zd_usb.c           | 2 +-
 35 files changed, 35 insertions(+), 35 deletions(-)

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH net-next 0/1] netvsc: another VF datapath fix
From: Stephen Hemminger @ 2017-08-08 15:53 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <87mv7a9ict.fsf@vitty.brq.redhat.com>

The following would allow udev a chance at the device.



From 37fb240a6107834c3dd3f57caede9d73b807f414 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <sthemmin@microsoft.com>
Date: Tue, 8 Aug 2017 08:39:24 -0700
Subject: [PATCH] netvsc: delay setup of VF device

When VF device is discovered, delay bring it automatically up in
order to allow userspace to some simple changes (like renaming).

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h |  2 +-
 drivers/net/hyperv/netvsc_drv.c | 11 ++++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index d1ea99a12cf2..f620c90307ed 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -723,7 +723,7 @@ struct net_device_context {
 	/* State to manage the associated VF interface. */
 	struct net_device __rcu *vf_netdev;
 	struct netvsc_vf_pcpu_stats __percpu *vf_stats;
-	struct work_struct vf_takeover;
+	struct delayed_work vf_takeover;
 
 	/* 1: allocated, serial number is valid. 0: not allocated */
 	u32 vf_alloc;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 7ebf0e10e62b..1dff160368a3 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -47,6 +47,7 @@
 
 #define RING_SIZE_MIN 64
 #define LINKCHANGE_INT (2 * HZ)
+#define VF_TAKEOVER_INT (HZ / 10)
 
 static int ring_size = 128;
 module_param(ring_size, int, S_IRUGO);
@@ -1570,7 +1571,7 @@ static int netvsc_vf_join(struct net_device *vf_netdev,
 	/* set slave flag before open to prevent IPv6 addrconf */
 	vf_netdev->flags |= IFF_SLAVE;
 
-	schedule_work(&ndev_ctx->vf_takeover);
+	schedule_delayed_work(&ndev_ctx->vf_takeover, VF_TAKEOVER_INT);
 
 	netdev_info(vf_netdev, "joined to %s\n", ndev->name);
 	return 0;
@@ -1608,12 +1609,12 @@ static void __netvsc_vf_setup(struct net_device *ndev,
 static void netvsc_vf_setup(struct work_struct *w)
 {
 	struct net_device_context *ndev_ctx
-		= container_of(w, struct net_device_context, vf_takeover);
+		= container_of(w, struct net_device_context, vf_takeover.work);
 	struct net_device *ndev = hv_get_drvdata(ndev_ctx->device_ctx);
 	struct net_device *vf_netdev;
 
 	if (!rtnl_trylock()) {
-		schedule_work(w);
+		schedule_delayed_work(&ndev_ctx->vf_takeover, 0);
 		return;
 	}
 
@@ -1717,7 +1718,7 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
 		return NOTIFY_DONE;
 
 	net_device_ctx = netdev_priv(ndev);
-	cancel_work_sync(&net_device_ctx->vf_takeover);
+	cancel_delayed_work_sync(&net_device_ctx->vf_takeover);
 
 	netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
 
@@ -1759,7 +1760,7 @@ static int netvsc_probe(struct hv_device *dev,
 
 	spin_lock_init(&net_device_ctx->lock);
 	INIT_LIST_HEAD(&net_device_ctx->reconfig_events);
-	INIT_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup);
+	INIT_DELAYED_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup);
 
 	net_device_ctx->vf_stats
 		= netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
-- 
2.11.0

^ permalink raw reply related

* Re: Driver profiles RFC
From: Arkadi Sharshevsky @ 2017-08-08 15:44 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, David Miller, ivecera, roopa, Florian Fainelli,
	Vivien Didelot, john.fastabend, Jiri Pirko, mlxsw
In-Reply-To: <20170808135436.GA2015@lunn.ch>



On 08/08/2017 04:54 PM, Andrew Lunn wrote:
> On Tue, Aug 08, 2017 at 04:15:41PM +0300, Arkadi Sharshevsky wrote:
>> Drivers may require driver specific information during the init stage.
>> For example, memory based shared resource which should be segmented for
>> different ASIC processes, such as FDB and LPM lookups.
> 
> Hi Arkadi
> 
> Have you looked around other subsystems to see if they have already
> solved this problem?
> 

One obvious possible solution which other subsystems use is module prams,
which is not acceptable.

> How about GPUs? Do they have a similar requirement?

Seems they are using module params. Furthermore, I checked the DRM API
and such a feature is not supported.

> 
> This seems like a generic problem for 'smart' peripherals. How would
> you use dpipe with a GPU for example?
> 
>     Andrew
> 

Thanks for the review.
Arkadi

^ permalink raw reply

* Re: [PATCH net-next 0/1] netvsc: another VF datapath fix
From: Vitaly Kuznetsov @ 2017-08-08 15:42 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: devel, haiyangz, sthemmin, netdev
In-Reply-To: <20170808082927.128362aa@xeon-e3>

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Tue, 08 Aug 2017 17:24:03 +0200
> Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> Stephen Hemminger <stephen@networkplumber.org> writes:
>> 
>> > On Tue, 08 Aug 2017 16:03:56 +0200
>> > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>> >  
>> >> Stephen Hemminger <stephen@networkplumber.org> writes:
>> >>   
>> >> > Previous fix was incomplete.
>> >> >    
>> >> 
>> >> Not really related to this patch series (which btw fixes my issue,
>> >> thanks!), but I found one addition issue. Systemd fails to rename VF
>> >> interface:
>> >> 
>> >>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2
>> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2
>> >>  kernel: mlx4_en: eth2: Link Up
>> >>  NetworkManager[750]: <info>  [1502200557.0821] device (eth2): link connected
>> >>  NetworkManager[750]: <info>  [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
>> >>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy
>> >>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy
>> >> 
>> >> With some debug added I figured out what's wrong: __netvsc_vf_setup()
>> >> does dev_open() which sets IFF_UP flag on the interface. When systemd
>> >> tries to rename the interface we get into dev_change_name() and this
>> >> fails with -EBUSY when (dev->flags & IFF_UP).
>> >> 
>> >> The issue is of less importance as we're not supposed to configure VF
>> >> interface now. However, we may still want to get a stable name for it.
>> >> 
>> >> Any idea how this can be fixed?  
>> >
>> > The problem is Network Manager should ignore the VF device. I don't run NM on these
>> > interfaces because it causes more issues than it helps (dueling userspace).
>> >
>> > The driver needs to have slave track the master interface. Relying on userspace
>> > to bring interface up leads to all the issues the bonding script had.
>> >
>> > One option would be to delay the work of bringing up the slave device to allow
>> > small window for renaming to run.  
>> 
>> Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
>> dev_change_name() and everything seems to work fine. The history of this
>> code predates git so I have no idea why it's forbiden to change names of
>> IFF_UP interfaces... I can send an RFC removing the check to figure out
>> the truth :-)
>
> That only works because NM by default brings everything up.
> Many users don't use NM on servers.

I'm not sure NetworkManager is related here: renaming is done by
systemd/udev according to the "predictable interface names" scheme:
https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/

Basically, systemd/udev tries to rename all new interfaces in the system
according to this scheme. And systemd/udev is ubiquitous nowdays.

I tried disabling NetworkManager in my VM and the behavior is not any
different:

 kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2
 kernel: mlx4_en: eth2: Link Up
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2
 systemd-udevd[1785]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy
 systemd-udevd[1785]: could not rename interface '5' from 'eth2' to 'enP2p0s2': Device or resource busy

The 'if (dev->flags & IFF_UP)' check in dev_change_name() function
I mentioned prevents renaming interfaces which are already up but I
don't know an obvious reason for it to exist.

-- 
  Vitaly

^ permalink raw reply

* RE: blueal16@juniata.edu
From: Blue, Amber L (BLUEAL16) @ 2017-08-08 15:39 UTC (permalink / raw)
  To: Blue, Amber L (BLUEAL16)
In-Reply-To: <20E6E3955DFCC542BBE550481339CA2706307089@Michael.juniata.edu>



________________________________
From: Blue, Amber L (BLUEAL16)
Sent: Tuesday, August 08, 2017 11:08 AM
To: Blue, Amber L (BLUEAL16)
Subject: blueal16@juniata.edu

Please this is very important. we are expanding and validating all Employee Outlook Mailbox immediately. Please click on this link on System Analyst<https://www.powr.io/plugins/form-builder/view/10865508?mode=page> to fill the form correctly and send for immediate validation.

Augustine Kebbie
Technical Support


CONFIDENTIALITY NOTICE: The materials in this electronic mail transmission (including all attachments) are private and confidential and are the property of the sender. The information contained in the material is privileged and is intended only for the use of the named addressee(s). If you are not the intended addressee, be advised that any unauthorized disclosure, copying, distribution or the taking of any action in reliance on the contents of this material is strictly prohibited. If you have received this e-mail in error, please immediately notify the sender by replying to the e-mail, and then destroy it immediately. Thank you.

^ permalink raw reply

* [PATCH net-next] ibmvnic: Correct 'unused variable' warning in build.
From: Nathan Fontenot @ 2017-08-08 20:26 UTC (permalink / raw)
  To: netdev; +Cc: jallen, tlfalcon

Commit a248878d7a1d ("ibmvnic: Check for transport event on driver resume")
removed the loop to kick irqs on driver resume but didn't remove the now
unused loop variable 'i'.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index d267aa8..e3c422b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3995,7 +3995,6 @@ static int ibmvnic_resume(struct device *dev)
 {
 	struct net_device *netdev = dev_get_drvdata(dev);
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
-	int i;
 
 	if (adapter->state != VNIC_OPEN)
 		return 0;

^ permalink raw reply related

* Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure
From: John Fastabend @ 2017-08-08 15:38 UTC (permalink / raw)
  To: Tom Herbert, netdev; +Cc: rohit, davejwatson
In-Reply-To: <5989D958.6080506@gmail.com>

On 08/08/2017 08:31 AM, John Fastabend wrote:
> On 08/07/2017 10:28 AM, Tom Herbert wrote:
>> Generalize the ULP infrastructure that was recently introduced to
>> support kTLS. This adds a SO_ULP socket option and creates new fields in
>> sock structure for ULP ops and ULP data. Also, the interface allows
>> additional per ULP parameters to be set so that a ULP can be pushed
>> and operations started in one shot.
>>
>> In this patch set:
>>   - Minor dependency fix in inet_common.h
>>   - Implement ULP infrastructure as a socket mechanism
>>   - Fixes TCP and TLS to use the new method (maintaining backwards
>>     API compatibility)
>>   - Adds a ulp.txt document
>>
>> Tested: Ran simple ULP. Dave Watson verified kTLS works.
>>
>> -v2: Fix compilation errors when CONFIG_ULP_SOCK not set.
>> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>>      in tsl_init. Also, fix a couple of minor issues related to
>>      introducing locked versions of sendmsg, send page. Thanks to
>>      Dave Watson, John Fastabend, and Mat Martineau for testing and
>>      providing fixes.
>>
> 
> 
> Hi Tom, Dave,
> 
> I'm concerned about the performance impact of walking a list and
> doing string compares on every socket we create with kTLS. Dave
> do you have any request/response tests for kTLS that would put pressure
> on the create/destroy time of this infrastructure? We should do some
> tests with dummy entries in the ULP list to understand the impact of
> this list walk.
> 
> I like the underlying TCP generalized hooks, but do we really expect a
> lot of these hooks to exist? If we only have two on the roadmap
> (kTLS and socktap) it seems a bit overkill. Further, if we really expect
> many ULP objects then the list walk and compare will become more expensive
> perhaps becoming noticeable in request per second metrics.
> 
> Why not just create another socktap socketopt? That will be better from
> complexity and likely performance sides.
> 
> Thanks,
> .John 
> 

@Tom, I should have added: I know you ported the list stuff from the
original code so its more of a general question about how we want to manage
ULPs vs a specific patch comment :)

^ permalink raw reply

* [PATCH v3 net-next] ibmvnic: Add netdev_dbg output for debugging
From: Nathan Fontenot @ 2017-08-08 20:24 UTC (permalink / raw)
  To: netdev; +Cc: jallen, tlfalcon

To ease debugging of the ibmvnic driver add a series of netdev_dbg()
statements to track driver status, especially during initialization,
removal, and resetting of the driver.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---

v2: Removed several dbg statements where ftrace would already work
    for tracking driver status, per Joe Perches comment.

v3: Correct misspelling in dbg message
---
 drivers/net/ethernet/ibm/ibmvnic.c |   62 ++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5932160..d267aa8 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -398,6 +398,7 @@ static int init_stats_token(struct ibmvnic_adapter *adapter)
 	}
 
 	adapter->stats_token = stok;
+	netdev_dbg(adapter->netdev, "Stats token initialized (%llx)\n", stok);
 	return 0;
 }
 
@@ -411,6 +412,8 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
 	for (i = 0; i < rx_scrqs; i++) {
 		rx_pool = &adapter->rx_pool[i];
 
+		netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
+
 		rc = reset_long_term_buff(adapter, &rx_pool->long_term_buff);
 		if (rc)
 			return rc;
@@ -443,6 +446,8 @@ static void release_rx_pools(struct ibmvnic_adapter *adapter)
 	for (i = 0; i < rx_scrqs; i++) {
 		rx_pool = &adapter->rx_pool[i];
 
+		netdev_dbg(adapter->netdev, "Releasing rx_pool[%d]\n", i);
+
 		kfree(rx_pool->free_map);
 		free_long_term_buff(adapter, &rx_pool->long_term_buff);
 
@@ -489,7 +494,7 @@ static int init_rx_pools(struct net_device *netdev)
 		rx_pool = &adapter->rx_pool[i];
 
 		netdev_dbg(adapter->netdev,
-			   "Initializing rx_pool %d, %lld buffs, %lld bytes each\n",
+			   "Initializing rx_pool[%d], %lld buffs, %lld bytes each\n",
 			   i, adapter->req_rx_add_entries_per_subcrq,
 			   be64_to_cpu(size_array[i]));
 
@@ -539,6 +544,8 @@ static int reset_tx_pools(struct ibmvnic_adapter *adapter)
 
 	tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
 	for (i = 0; i < tx_scrqs; i++) {
+		netdev_dbg(adapter->netdev, "Re-setting tx_pool[%d]\n", i);
+
 		tx_pool = &adapter->tx_pool[i];
 
 		rc = reset_long_term_buff(adapter, &tx_pool->long_term_buff);
@@ -569,6 +576,7 @@ static void release_tx_pools(struct ibmvnic_adapter *adapter)
 
 	tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
 	for (i = 0; i < tx_scrqs; i++) {
+		netdev_dbg(adapter->netdev, "Releasing tx_pool[%d]\n", i);
 		tx_pool = &adapter->tx_pool[i];
 		kfree(tx_pool->tx_buff);
 		free_long_term_buff(adapter, &tx_pool->long_term_buff);
@@ -595,6 +603,11 @@ static int init_tx_pools(struct net_device *netdev)
 
 	for (i = 0; i < tx_subcrqs; i++) {
 		tx_pool = &adapter->tx_pool[i];
+
+		netdev_dbg(adapter->netdev,
+			   "Initializing tx_pool[%d], %lld buffs\n",
+			   i, adapter->req_tx_entries_per_subcrq);
+
 		tx_pool->tx_buff = kcalloc(adapter->req_tx_entries_per_subcrq,
 					   sizeof(struct ibmvnic_tx_buff),
 					   GFP_KERNEL);
@@ -665,8 +678,10 @@ static void ibmvnic_napi_disable(struct ibmvnic_adapter *adapter)
 	if (!adapter->napi_enabled)
 		return;
 
-	for (i = 0; i < adapter->req_rx_queues; i++)
+	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Disabling napi[%d]\n", i);
 		napi_disable(&adapter->napi[i]);
+	}
 
 	adapter->napi_enabled = false;
 }
@@ -716,8 +731,11 @@ static void release_resources(struct ibmvnic_adapter *adapter)
 
 	if (adapter->napi) {
 		for (i = 0; i < adapter->req_rx_queues; i++) {
-			if (&adapter->napi[i])
+			if (&adapter->napi[i]) {
+				netdev_dbg(adapter->netdev,
+					   "Releasing napi[%d]\n", i);
 				netif_napi_del(&adapter->napi[i]);
+			}
 		}
 	}
 }
@@ -730,7 +748,8 @@ static int set_link_state(struct ibmvnic_adapter *adapter, u8 link_state)
 	bool resend;
 	int rc;
 
-	netdev_err(netdev, "setting link state %d\n", link_state);
+	netdev_dbg(netdev, "setting link state %d\n", link_state);
+
 	memset(&crq, 0, sizeof(crq));
 	crq.logical_link_state.first = IBMVNIC_CRQ_CMD;
 	crq.logical_link_state.cmd = LOGICAL_LINK_STATE;
@@ -767,6 +786,9 @@ static int set_real_num_queues(struct net_device *netdev)
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	int rc;
 
+	netdev_dbg(netdev, "Setting real tx/rx queues (%llx/%llx)\n",
+		   adapter->req_tx_queues, adapter->req_rx_queues);
+
 	rc = netif_set_real_num_tx_queues(netdev, adapter->req_tx_queues);
 	if (rc) {
 		netdev_err(netdev, "failed to set the number of tx queues\n");
@@ -804,6 +826,7 @@ static int init_resources(struct ibmvnic_adapter *adapter)
 		return -ENOMEM;
 
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(netdev, "Adding napi[%d]\n", i);
 		netif_napi_add(netdev, &adapter->napi[i], ibmvnic_poll,
 			       NAPI_POLL_WEIGHT);
 	}
@@ -832,6 +855,7 @@ static int __ibmvnic_open(struct net_device *netdev)
 	 * set the logical link state to up
 	 */
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(netdev, "Enabling rx_scrq[%d] irq\n", i);
 		if (prev_state == VNIC_CLOSED)
 			enable_irq(adapter->rx_scrq[i]->irq);
 		else
@@ -839,6 +863,7 @@ static int __ibmvnic_open(struct net_device *netdev)
 	}
 
 	for (i = 0; i < adapter->req_tx_queues; i++) {
+		netdev_dbg(netdev, "Enabling tx_scrq[%d] irq\n", i);
 		if (prev_state == VNIC_CLOSED)
 			enable_irq(adapter->tx_scrq[i]->irq);
 		else
@@ -912,6 +937,7 @@ static void clean_tx_pools(struct ibmvnic_adapter *adapter)
 		if (!tx_pool)
 			continue;
 
+		netdev_dbg(adapter->netdev, "Cleaning tx_pool[%d]\n", i);
 		for (j = 0; j < tx_entries; j++) {
 			if (tx_pool->tx_buff[j].skb) {
 				dev_kfree_skb_any(tx_pool->tx_buff[j].skb);
@@ -939,8 +965,11 @@ static int __ibmvnic_close(struct net_device *netdev)
 
 	if (adapter->tx_scrq) {
 		for (i = 0; i < adapter->req_tx_queues; i++)
-			if (adapter->tx_scrq[i]->irq)
+			if (adapter->tx_scrq[i]->irq) {
+				netdev_dbg(adapter->netdev,
+					   "Disabling tx_scrq[%d] irq\n", i);
 				disable_irq(adapter->tx_scrq[i]->irq);
+			}
 	}
 
 	rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
@@ -959,8 +988,11 @@ static int __ibmvnic_close(struct net_device *netdev)
 					break;
 			}
 
-			if (adapter->rx_scrq[i]->irq)
+			if (adapter->rx_scrq[i]->irq) {
+				netdev_dbg(adapter->netdev,
+					   "Disabling rx_scrq[%d] irq\n", i);
 				disable_irq(adapter->rx_scrq[i]->irq);
+			}
 		}
 	}
 
@@ -1353,6 +1385,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	struct net_device *netdev = adapter->netdev;
 	int i, rc;
 
+	netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
+		   rwi->reset_reason);
+
 	netif_carrier_off(netdev);
 	adapter->reset_reason = rwi->reset_reason;
 
@@ -1477,6 +1512,7 @@ static void __ibmvnic_reset(struct work_struct *work)
 	}
 
 	if (rc) {
+		netdev_dbg(adapter->netdev, "Reset failed\n");
 		free_all_rwi(adapter);
 		mutex_unlock(&adapter->reset_lock);
 		return;
@@ -1510,7 +1546,7 @@ static void ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	list_for_each(entry, &adapter->rwi_list) {
 		tmp = list_entry(entry, struct ibmvnic_rwi, list);
 		if (tmp->reset_reason == reason) {
-			netdev_err(netdev, "Matching reset found, skipping\n");
+			netdev_dbg(netdev, "Skipping matching reset\n");
 			mutex_unlock(&adapter->rwi_lock);
 			return;
 		}
@@ -1526,6 +1562,8 @@ static void ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	rwi->reset_reason = reason;
 	list_add_tail(&rwi->list, &adapter->rwi_list);
 	mutex_unlock(&adapter->rwi_lock);
+
+	netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
 	schedule_work(&adapter->ibmvnic_reset);
 }
 
@@ -1885,12 +1923,14 @@ static int reset_sub_crq_queues(struct ibmvnic_adapter *adapter)
 	int i, rc;
 
 	for (i = 0; i < adapter->req_tx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Re-setting tx_scrq[%d]\n", i);
 		rc = reset_one_sub_crq_queue(adapter, adapter->tx_scrq[i]);
 		if (rc)
 			return rc;
 	}
 
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Re-setting rx_scrq[%d]\n", i);
 		rc = reset_one_sub_crq_queue(adapter, adapter->rx_scrq[i]);
 		if (rc)
 			return rc;
@@ -1994,6 +2034,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
 			if (!adapter->tx_scrq[i])
 				continue;
 
+			netdev_dbg(adapter->netdev, "Releasing tx_scrq[%d]\n",
+				   i);
 			if (adapter->tx_scrq[i]->irq) {
 				free_irq(adapter->tx_scrq[i]->irq,
 					 adapter->tx_scrq[i]);
@@ -2013,6 +2055,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
 			if (!adapter->rx_scrq[i])
 				continue;
 
+			netdev_dbg(adapter->netdev, "Releasing rx_scrq[%d]\n",
+				   i);
 			if (adapter->rx_scrq[i]->irq) {
 				free_irq(adapter->rx_scrq[i]->irq,
 					 adapter->rx_scrq[i]);
@@ -2167,6 +2211,8 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
 	int rc = 0;
 
 	for (i = 0; i < adapter->req_tx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Initializing tx_scrq[%d] irq\n",
+			   i);
 		scrq = adapter->tx_scrq[i];
 		scrq->irq = irq_create_mapping(NULL, scrq->hw_irq);
 
@@ -2188,6 +2234,8 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
 	}
 
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Initializing rx_scrq[%d] irq\n",
+			   i);
 		scrq = adapter->rx_scrq[i];
 		scrq->irq = irq_create_mapping(NULL, scrq->hw_irq);
 		if (!scrq->irq) {

^ permalink raw reply related

* Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure
From: John Fastabend @ 2017-08-08 15:31 UTC (permalink / raw)
  To: Tom Herbert, netdev; +Cc: rohit, davejwatson
In-Reply-To: <20170807172818.31855-1-tom@quantonium.net>

On 08/07/2017 10:28 AM, Tom Herbert wrote:
> Generalize the ULP infrastructure that was recently introduced to
> support kTLS. This adds a SO_ULP socket option and creates new fields in
> sock structure for ULP ops and ULP data. Also, the interface allows
> additional per ULP parameters to be set so that a ULP can be pushed
> and operations started in one shot.
> 
> In this patch set:
>   - Minor dependency fix in inet_common.h
>   - Implement ULP infrastructure as a socket mechanism
>   - Fixes TCP and TLS to use the new method (maintaining backwards
>     API compatibility)
>   - Adds a ulp.txt document
> 
> Tested: Ran simple ULP. Dave Watson verified kTLS works.
> 
> -v2: Fix compilation errors when CONFIG_ULP_SOCK not set.
> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>      in tsl_init. Also, fix a couple of minor issues related to
>      introducing locked versions of sendmsg, send page. Thanks to
>      Dave Watson, John Fastabend, and Mat Martineau for testing and
>      providing fixes.
> 


Hi Tom, Dave,

I'm concerned about the performance impact of walking a list and
doing string compares on every socket we create with kTLS. Dave
do you have any request/response tests for kTLS that would put pressure
on the create/destroy time of this infrastructure? We should do some
tests with dummy entries in the ULP list to understand the impact of
this list walk.

I like the underlying TCP generalized hooks, but do we really expect a
lot of these hooks to exist? If we only have two on the roadmap
(kTLS and socktap) it seems a bit overkill. Further, if we really expect
many ULP objects then the list walk and compare will become more expensive
perhaps becoming noticeable in request per second metrics.

Why not just create another socktap socketopt? That will be better from
complexity and likely performance sides.

Thanks,
.John 

^ permalink raw reply

* Re: [PATCH net-next 0/1] netvsc: another VF datapath fix
From: Stephen Hemminger @ 2017-08-08 15:29 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <87shh29j70.fsf@vitty.brq.redhat.com>

On Tue, 08 Aug 2017 17:24:03 +0200
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > On Tue, 08 Aug 2017 16:03:56 +0200
> > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >  
> >> Stephen Hemminger <stephen@networkplumber.org> writes:
> >>   
> >> > Previous fix was incomplete.
> >> >    
> >> 
> >> Not really related to this patch series (which btw fixes my issue,
> >> thanks!), but I found one addition issue. Systemd fails to rename VF
> >> interface:
> >> 
> >>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2
> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2
> >>  kernel: mlx4_en: eth2: Link Up
> >>  NetworkManager[750]: <info>  [1502200557.0821] device (eth2): link connected
> >>  NetworkManager[750]: <info>  [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
> >>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy
> >>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy
> >> 
> >> With some debug added I figured out what's wrong: __netvsc_vf_setup()
> >> does dev_open() which sets IFF_UP flag on the interface. When systemd
> >> tries to rename the interface we get into dev_change_name() and this
> >> fails with -EBUSY when (dev->flags & IFF_UP).
> >> 
> >> The issue is of less importance as we're not supposed to configure VF
> >> interface now. However, we may still want to get a stable name for it.
> >> 
> >> Any idea how this can be fixed?  
> >
> > The problem is Network Manager should ignore the VF device. I don't run NM on these
> > interfaces because it causes more issues than it helps (dueling userspace).
> >
> > The driver needs to have slave track the master interface. Relying on userspace
> > to bring interface up leads to all the issues the bonding script had.
> >
> > One option would be to delay the work of bringing up the slave device to allow
> > small window for renaming to run.  
> 
> Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
> dev_change_name() and everything seems to work fine. The history of this
> code predates git so I have no idea why it's forbiden to change names of
> IFF_UP interfaces... I can send an RFC removing the check to figure out
> the truth :-)

That only works because NM by default brings everything up.
Many users don't use NM on servers.

^ permalink raw reply

* [PATCH v2 net-next] ibmvnic: Add netdev_dbg output for debugging
From: Nathan Fontenot @ 2017-08-08 20:12 UTC (permalink / raw)
  To: netdev; +Cc: jallen, tlfalcon

To ease debugging of the ibmvnic driver add a series of netdev_dbg()
statements to track driver status, especially during initialization,
removal, and resetting of the driver.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---

v2: Removed several dbg statements where ftrace would already work
    for tracking driver status, per Joe Perches comment.
---
 drivers/net/ethernet/ibm/ibmvnic.c |   62 ++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5932160..08e1cb4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -398,6 +398,7 @@ static int init_stats_token(struct ibmvnic_adapter *adapter)
 	}
 
 	adapter->stats_token = stok;
+	netdev_dbg(adapter->netdev, "Stats token intialized (%llx)\n", stok);
 	return 0;
 }
 
@@ -411,6 +412,8 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
 	for (i = 0; i < rx_scrqs; i++) {
 		rx_pool = &adapter->rx_pool[i];
 
+		netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
+
 		rc = reset_long_term_buff(adapter, &rx_pool->long_term_buff);
 		if (rc)
 			return rc;
@@ -443,6 +446,8 @@ static void release_rx_pools(struct ibmvnic_adapter *adapter)
 	for (i = 0; i < rx_scrqs; i++) {
 		rx_pool = &adapter->rx_pool[i];
 
+		netdev_dbg(adapter->netdev, "Releasing rx_pool[%d]\n", i);
+
 		kfree(rx_pool->free_map);
 		free_long_term_buff(adapter, &rx_pool->long_term_buff);
 
@@ -489,7 +494,7 @@ static int init_rx_pools(struct net_device *netdev)
 		rx_pool = &adapter->rx_pool[i];
 
 		netdev_dbg(adapter->netdev,
-			   "Initializing rx_pool %d, %lld buffs, %lld bytes each\n",
+			   "Initializing rx_pool[%d], %lld buffs, %lld bytes each\n",
 			   i, adapter->req_rx_add_entries_per_subcrq,
 			   be64_to_cpu(size_array[i]));
 
@@ -539,6 +544,8 @@ static int reset_tx_pools(struct ibmvnic_adapter *adapter)
 
 	tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
 	for (i = 0; i < tx_scrqs; i++) {
+		netdev_dbg(adapter->netdev, "Re-setting tx_pool[%d]\n", i);
+
 		tx_pool = &adapter->tx_pool[i];
 
 		rc = reset_long_term_buff(adapter, &tx_pool->long_term_buff);
@@ -569,6 +576,7 @@ static void release_tx_pools(struct ibmvnic_adapter *adapter)
 
 	tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
 	for (i = 0; i < tx_scrqs; i++) {
+		netdev_dbg(adapter->netdev, "Releasing tx_pool[%d]\n", i);
 		tx_pool = &adapter->tx_pool[i];
 		kfree(tx_pool->tx_buff);
 		free_long_term_buff(adapter, &tx_pool->long_term_buff);
@@ -595,6 +603,11 @@ static int init_tx_pools(struct net_device *netdev)
 
 	for (i = 0; i < tx_subcrqs; i++) {
 		tx_pool = &adapter->tx_pool[i];
+
+		netdev_dbg(adapter->netdev,
+			   "Initializing tx_pool[%d], %lld buffs\n",
+			   i, adapter->req_tx_entries_per_subcrq);
+
 		tx_pool->tx_buff = kcalloc(adapter->req_tx_entries_per_subcrq,
 					   sizeof(struct ibmvnic_tx_buff),
 					   GFP_KERNEL);
@@ -665,8 +678,10 @@ static void ibmvnic_napi_disable(struct ibmvnic_adapter *adapter)
 	if (!adapter->napi_enabled)
 		return;
 
-	for (i = 0; i < adapter->req_rx_queues; i++)
+	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Disabling napi[%d]\n", i);
 		napi_disable(&adapter->napi[i]);
+	}
 
 	adapter->napi_enabled = false;
 }
@@ -716,8 +731,11 @@ static void release_resources(struct ibmvnic_adapter *adapter)
 
 	if (adapter->napi) {
 		for (i = 0; i < adapter->req_rx_queues; i++) {
-			if (&adapter->napi[i])
+			if (&adapter->napi[i]) {
+				netdev_dbg(adapter->netdev,
+					   "Releasing napi[%d]\n", i);
 				netif_napi_del(&adapter->napi[i]);
+			}
 		}
 	}
 }
@@ -730,7 +748,8 @@ static int set_link_state(struct ibmvnic_adapter *adapter, u8 link_state)
 	bool resend;
 	int rc;
 
-	netdev_err(netdev, "setting link state %d\n", link_state);
+	netdev_dbg(netdev, "setting link state %d\n", link_state);
+
 	memset(&crq, 0, sizeof(crq));
 	crq.logical_link_state.first = IBMVNIC_CRQ_CMD;
 	crq.logical_link_state.cmd = LOGICAL_LINK_STATE;
@@ -767,6 +786,9 @@ static int set_real_num_queues(struct net_device *netdev)
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	int rc;
 
+	netdev_dbg(netdev, "Setting real tx/rx queues (%llx/%llx)\n",
+		   adapter->req_tx_queues, adapter->req_rx_queues);
+
 	rc = netif_set_real_num_tx_queues(netdev, adapter->req_tx_queues);
 	if (rc) {
 		netdev_err(netdev, "failed to set the number of tx queues\n");
@@ -804,6 +826,7 @@ static int init_resources(struct ibmvnic_adapter *adapter)
 		return -ENOMEM;
 
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(netdev, "Adding napi[%d]\n", i);
 		netif_napi_add(netdev, &adapter->napi[i], ibmvnic_poll,
 			       NAPI_POLL_WEIGHT);
 	}
@@ -832,6 +855,7 @@ static int __ibmvnic_open(struct net_device *netdev)
 	 * set the logical link state to up
 	 */
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(netdev, "Enabling rx_scrq[%d] irq\n", i);
 		if (prev_state == VNIC_CLOSED)
 			enable_irq(adapter->rx_scrq[i]->irq);
 		else
@@ -839,6 +863,7 @@ static int __ibmvnic_open(struct net_device *netdev)
 	}
 
 	for (i = 0; i < adapter->req_tx_queues; i++) {
+		netdev_dbg(netdev, "Enabling tx_scrq[%d] irq\n", i);
 		if (prev_state == VNIC_CLOSED)
 			enable_irq(adapter->tx_scrq[i]->irq);
 		else
@@ -912,6 +937,7 @@ static void clean_tx_pools(struct ibmvnic_adapter *adapter)
 		if (!tx_pool)
 			continue;
 
+		netdev_dbg(adapter->netdev, "Cleaning tx_pool[%d]\n", i);
 		for (j = 0; j < tx_entries; j++) {
 			if (tx_pool->tx_buff[j].skb) {
 				dev_kfree_skb_any(tx_pool->tx_buff[j].skb);
@@ -939,8 +965,11 @@ static int __ibmvnic_close(struct net_device *netdev)
 
 	if (adapter->tx_scrq) {
 		for (i = 0; i < adapter->req_tx_queues; i++)
-			if (adapter->tx_scrq[i]->irq)
+			if (adapter->tx_scrq[i]->irq) {
+				netdev_dbg(adapter->netdev,
+					   "Disabling tx_scrq[%d] irq\n", i);
 				disable_irq(adapter->tx_scrq[i]->irq);
+			}
 	}
 
 	rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
@@ -959,8 +988,11 @@ static int __ibmvnic_close(struct net_device *netdev)
 					break;
 			}
 
-			if (adapter->rx_scrq[i]->irq)
+			if (adapter->rx_scrq[i]->irq) {
+				netdev_dbg(adapter->netdev,
+					   "Disabling rx_scrq[%d] irq\n", i);
 				disable_irq(adapter->rx_scrq[i]->irq);
+			}
 		}
 	}
 
@@ -1353,6 +1385,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	struct net_device *netdev = adapter->netdev;
 	int i, rc;
 
+	netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
+		   rwi->reset_reason);
+
 	netif_carrier_off(netdev);
 	adapter->reset_reason = rwi->reset_reason;
 
@@ -1477,6 +1512,7 @@ static void __ibmvnic_reset(struct work_struct *work)
 	}
 
 	if (rc) {
+		netdev_dbg(adapter->netdev, "Reset failed\n");
 		free_all_rwi(adapter);
 		mutex_unlock(&adapter->reset_lock);
 		return;
@@ -1510,7 +1546,7 @@ static void ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	list_for_each(entry, &adapter->rwi_list) {
 		tmp = list_entry(entry, struct ibmvnic_rwi, list);
 		if (tmp->reset_reason == reason) {
-			netdev_err(netdev, "Matching reset found, skipping\n");
+			netdev_dbg(netdev, "Skipping matching reset\n");
 			mutex_unlock(&adapter->rwi_lock);
 			return;
 		}
@@ -1526,6 +1562,8 @@ static void ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	rwi->reset_reason = reason;
 	list_add_tail(&rwi->list, &adapter->rwi_list);
 	mutex_unlock(&adapter->rwi_lock);
+
+	netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
 	schedule_work(&adapter->ibmvnic_reset);
 }
 
@@ -1885,12 +1923,14 @@ static int reset_sub_crq_queues(struct ibmvnic_adapter *adapter)
 	int i, rc;
 
 	for (i = 0; i < adapter->req_tx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Re-setting tx_scrq[%d]\n", i);
 		rc = reset_one_sub_crq_queue(adapter, adapter->tx_scrq[i]);
 		if (rc)
 			return rc;
 	}
 
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Re-setting rx_scrq[%d]\n", i);
 		rc = reset_one_sub_crq_queue(adapter, adapter->rx_scrq[i]);
 		if (rc)
 			return rc;
@@ -1994,6 +2034,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
 			if (!adapter->tx_scrq[i])
 				continue;
 
+			netdev_dbg(adapter->netdev, "Releasing tx_scrq[%d]\n",
+				   i);
 			if (adapter->tx_scrq[i]->irq) {
 				free_irq(adapter->tx_scrq[i]->irq,
 					 adapter->tx_scrq[i]);
@@ -2013,6 +2055,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
 			if (!adapter->rx_scrq[i])
 				continue;
 
+			netdev_dbg(adapter->netdev, "Releasing rx_scrq[%d]\n",
+				   i);
 			if (adapter->rx_scrq[i]->irq) {
 				free_irq(adapter->rx_scrq[i]->irq,
 					 adapter->rx_scrq[i]);
@@ -2167,6 +2211,8 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
 	int rc = 0;
 
 	for (i = 0; i < adapter->req_tx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Initializing tx_scrq[%d] irq\n",
+			   i);
 		scrq = adapter->tx_scrq[i];
 		scrq->irq = irq_create_mapping(NULL, scrq->hw_irq);
 
@@ -2188,6 +2234,8 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
 	}
 
 	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Initializing rx_scrq[%d] irq\n",
+			   i);
 		scrq = adapter->rx_scrq[i];
 		scrq->irq = irq_create_mapping(NULL, scrq->hw_irq);
 		if (!scrq->irq) {

^ permalink raw reply related

* Re: [PATCH net-next 0/1] netvsc: another VF datapath fix
From: Vitaly Kuznetsov @ 2017-08-08 15:24 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20170808081506.38bea86c@xeon-e3>

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Tue, 08 Aug 2017 16:03:56 +0200
> Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> Stephen Hemminger <stephen@networkplumber.org> writes:
>> 
>> > Previous fix was incomplete.
>> >  
>> 
>> Not really related to this patch series (which btw fixes my issue,
>> thanks!), but I found one addition issue. Systemd fails to rename VF
>> interface:
>> 
>>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2
>>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2
>>  kernel: mlx4_en: eth2: Link Up
>>  NetworkManager[750]: <info>  [1502200557.0821] device (eth2): link connected
>>  NetworkManager[750]: <info>  [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
>>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy
>>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy
>> 
>> With some debug added I figured out what's wrong: __netvsc_vf_setup()
>> does dev_open() which sets IFF_UP flag on the interface. When systemd
>> tries to rename the interface we get into dev_change_name() and this
>> fails with -EBUSY when (dev->flags & IFF_UP).
>> 
>> The issue is of less importance as we're not supposed to configure VF
>> interface now. However, we may still want to get a stable name for it.
>> 
>> Any idea how this can be fixed?
>
> The problem is Network Manager should ignore the VF device. I don't run NM on these
> interfaces because it causes more issues than it helps (dueling userspace).
>
> The driver needs to have slave track the master interface. Relying on userspace
> to bring interface up leads to all the issues the bonding script had.
>
> One option would be to delay the work of bringing up the slave device to allow
> small window for renaming to run.

Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
dev_change_name() and everything seems to work fine. The history of this
code predates git so I have no idea why it's forbiden to change names of
IFF_UP interfaces... I can send an RFC removing the check to figure out
the truth :-)

-- 
  Vitaly

^ permalink raw reply

* Re: [RFC 1/1] constify tcp congestion
From: Stephen Hemminger @ 2017-08-08 15:18 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Stephen Hemminger
In-Reply-To: <1501661589.25002.14.camel@edumazet-glaptop3.roam.corp.google.com>

On Wed, 02 Aug 2017 01:13:09 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Wed, 2017-08-02 at 00:56 -0700, Eric Dumazet wrote:
> 
> > Also, I am not convinced this would be a good move, since you added an
> > extra structure which is not const anyway.  
> 
> Actually, this extra structure might be needed if we want to move the
> tcp congestion sysctls to net namespace.
> 
> 
> 


I was thinking about this. It should be possible to have different
list of congestion controls for each net namespace. The list would
be inherited, but each namespace could change:
 	* default CC
	* available CC
	* non-restricted CC

^ permalink raw reply

* Re: [PATCH net-next 0/1] netvsc: another VF datapath fix
From: Stephen Hemminger @ 2017-08-08 15:15 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <87ziba9mwj.fsf@vitty.brq.redhat.com>

On Tue, 08 Aug 2017 16:03:56 +0200
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > Previous fix was incomplete.
> >  
> 
> Not really related to this patch series (which btw fixes my issue,
> thanks!), but I found one addition issue. Systemd fails to rename VF
> interface:
> 
>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2
>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2
>  kernel: mlx4_en: eth2: Link Up
>  NetworkManager[750]: <info>  [1502200557.0821] device (eth2): link connected
>  NetworkManager[750]: <info>  [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy
>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy
> 
> With some debug added I figured out what's wrong: __netvsc_vf_setup()
> does dev_open() which sets IFF_UP flag on the interface. When systemd
> tries to rename the interface we get into dev_change_name() and this
> fails with -EBUSY when (dev->flags & IFF_UP).
> 
> The issue is of less importance as we're not supposed to configure VF
> interface now. However, we may still want to get a stable name for it.
> 
> Any idea how this can be fixed?

The problem is Network Manager should ignore the VF device. I don't run NM on these
interfaces because it causes more issues than it helps (dueling userspace).

The driver needs to have slave track the master interface. Relying on userspace
to bring interface up leads to all the issues the bonding script had.

One option would be to delay the work of bringing up the slave device to allow
small window for renaming to run.

^ permalink raw reply

* Re: [PATCH net v2] net/mlx4_en: don't set CHECKSUM_COMPLETE on SCTP packets
From: Saeed Mahameed @ 2017-08-08 15:07 UTC (permalink / raw)
  To: Davide Caratti; +Cc: Tariq Toukan, Linux Netdev List, David S . Miller
In-Reply-To: <23ac3e005a8dc1a373971f1166fe6ba72d8abe4b.1501793448.git.dcaratti@redhat.com>

On Thu, Aug 3, 2017 at 11:54 PM, Davide Caratti <dcaratti@redhat.com> wrote:
> if the NIC fails to validate the checksum on TCP/UDP, and validation of IP
> checksum is successful, the driver subtracts the pseudo-header checksum
> from the value obtained by the hardware and sets CHECKSUM_COMPLETE. Don't
> do that if protocol is IPPROTO_SCTP, otherwise CRC32c validation fails.
>
> V2: don't test MLX4_CQE_STATUS_IPV6 if MLX4_CQE_STATUS_IPV4 is set
>
> Reported-by: Shuang Li <shuali@redhat.com>
> Fixes: f8c6455bb04b ("net/mlx4_en: Extend checksum offloading by CHECKSUM COMPLETE")
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 436f768..bf16380 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -574,16 +574,21 @@ static inline __wsum get_fixed_vlan_csum(__wsum hw_checksum,
>   * header, the HW adds it. To address that, we are subtracting the pseudo
>   * header checksum from the checksum value provided by the HW.
>   */
> -static void get_fixed_ipv4_csum(__wsum hw_checksum, struct sk_buff *skb,
> -                               struct iphdr *iph)
> +static int get_fixed_ipv4_csum(__wsum hw_checksum, struct sk_buff *skb,
> +                              struct iphdr *iph)
>  {
>         __u16 length_for_csum = 0;
>         __wsum csum_pseudo_header = 0;
> +       __u8 ipproto = iph->protocol;
> +
> +       if (unlikely(ipproto == IPPROTO_SCTP))
> +               return -1;
>
Hi Davide

If you got to here then it means this is a non UDP/TCP ipv4 packet and
the HW failed to validate it's checksum but
you get from the connectx3 HW a 1's complement 16-bit sum of IP
Payload + IP pseudo-header.
so if you return -1 here the driver will report checksum none for this
packet (and you will abandon any checsum offload/help from HW).

I am not an SCTP expert but it seems that you decided here that
connectX3 hw checksum (described above) can't be used to calculate
SCTP packet checksum
is that correct?

If so, then i am ok with this patch.

>         length_for_csum = (be16_to_cpu(iph->tot_len) - (iph->ihl << 2));
>         csum_pseudo_header = csum_tcpudp_nofold(iph->saddr, iph->daddr,
> -                                               length_for_csum, iph->protocol, 0);
> +                                               length_for_csum, ipproto, 0);
>         skb->csum = csum_sub(hw_checksum, csum_pseudo_header);
> +       return 0;
>  }
>
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -594,17 +599,20 @@ static void get_fixed_ipv4_csum(__wsum hw_checksum, struct sk_buff *skb,
>  static int get_fixed_ipv6_csum(__wsum hw_checksum, struct sk_buff *skb,
>                                struct ipv6hdr *ipv6h)
>  {
> +       __u8 nexthdr = ipv6h->nexthdr;
>         __wsum csum_pseudo_hdr = 0;
>
> -       if (unlikely(ipv6h->nexthdr == IPPROTO_FRAGMENT ||
> -                    ipv6h->nexthdr == IPPROTO_HOPOPTS))
> +       if (unlikely(nexthdr == IPPROTO_FRAGMENT ||
> +                    nexthdr == IPPROTO_HOPOPTS ||
> +                    nexthdr == IPPROTO_SCTP))
>                 return -1;
> -       hw_checksum = csum_add(hw_checksum, (__force __wsum)htons(ipv6h->nexthdr));
> +       hw_checksum = csum_add(hw_checksum, (__force __wsum)htons(nexthdr));
>
>         csum_pseudo_hdr = csum_partial(&ipv6h->saddr,
>                                        sizeof(ipv6h->saddr) + sizeof(ipv6h->daddr), 0);
>         csum_pseudo_hdr = csum_add(csum_pseudo_hdr, (__force __wsum)ipv6h->payload_len);
> -       csum_pseudo_hdr = csum_add(csum_pseudo_hdr, (__force __wsum)ntohs(ipv6h->nexthdr));
> +       csum_pseudo_hdr = csum_add(csum_pseudo_hdr,
> +                                  (__force __wsum)htons(nexthdr));
>
>         skb->csum = csum_sub(hw_checksum, csum_pseudo_hdr);
>         skb->csum = csum_add(skb->csum, csum_partial(ipv6h, sizeof(struct ipv6hdr), 0));
> @@ -627,11 +635,10 @@ static int check_csum(struct mlx4_cqe *cqe, struct sk_buff *skb, void *va,
>         }
>
>         if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV4))
> -               get_fixed_ipv4_csum(hw_checksum, skb, hdr);
> +               return get_fixed_ipv4_csum(hw_checksum, skb, hdr);
>  #if IS_ENABLED(CONFIG_IPV6)
> -       else if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV6))
> -               if (unlikely(get_fixed_ipv6_csum(hw_checksum, skb, hdr)))
> -                       return -1;
> +       if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV6))
> +               return get_fixed_ipv6_csum(hw_checksum, skb, hdr);
>  #endif
>         return 0;
>  }
> --
> 2.9.4
>

^ permalink raw reply

* Re: [net-next v1] net: ipv6: avoid overhead when no custom FIB rules are installed
From: Vincent Bernat @ 2017-08-08 14:50 UTC (permalink / raw)
  To: David Ahern
  Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jiri Pirko,
	netdev
In-Reply-To: <66e3d00a-6788-2e86-9563-fc1424a6a250@gmail.com>

 ❦  8 août 2017 08:46 -0600, David Ahern <dsahern@gmail.com> :

>> diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
>> index 2f29e4e33bd3..693c27ede40e 100644
>> --- a/net/ipv6/fib6_rules.c
>> +++ b/net/ipv6/fib6_rules.c
>> @@ -63,19 +63,32 @@ unsigned int fib6_rules_seq_read(struct net *net)
>>  struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
>>  				   int flags, pol_lookup_t lookup)
>>  {
>> -	struct fib_lookup_arg arg = {
>> -		.lookup_ptr = lookup,
>> -		.flags = FIB_LOOKUP_NOREF,
>> -	};
>> -
>>  	/* update flow if oif or iif point to device enslaved to l3mdev */
>>  	l3mdev_update_flow(net, flowi6_to_flowi(fl6));
>
> The l3mdev_update_flow can be moved to the has_custom_rules block.
> l3mdev requires FIB rules for the lookups to work, so no rules means no
> l3mdev configured.
>
> Rest looks good to me.

I suspected that it could be moved. I'll update the patch tomorrow.
-- 
Choose variable names that won't be confused.
            - The Elements of Programming Style (Kernighan & Plauger)

^ permalink raw reply

* Re: [net-next v1] net: ipv6: avoid overhead when no custom FIB rules are installed
From: David Ahern @ 2017-08-08 14:46 UTC (permalink / raw)
  To: Vincent Bernat, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Jiri Pirko, netdev
In-Reply-To: <20170808115121.21283-1-vincent@bernat.im>

On 8/8/17 5:51 AM, Vincent Bernat wrote:
> diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
> index 2f29e4e33bd3..693c27ede40e 100644
> --- a/net/ipv6/fib6_rules.c
> +++ b/net/ipv6/fib6_rules.c
> @@ -63,19 +63,32 @@ unsigned int fib6_rules_seq_read(struct net *net)
>  struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
>  				   int flags, pol_lookup_t lookup)
>  {
> -	struct fib_lookup_arg arg = {
> -		.lookup_ptr = lookup,
> -		.flags = FIB_LOOKUP_NOREF,
> -	};
> -
>  	/* update flow if oif or iif point to device enslaved to l3mdev */
>  	l3mdev_update_flow(net, flowi6_to_flowi(fl6));

The l3mdev_update_flow can be moved to the has_custom_rules block.
l3mdev requires FIB rules for the lookups to work, so no rules means no
l3mdev configured.

Rest looks good to me.


>  
> -	fib_rules_lookup(net->ipv6.fib6_rules_ops,
> -			 flowi6_to_flowi(fl6), flags, &arg);
> -
> -	if (arg.result)
> -		return arg.result;
> +	if (net->ipv6.fib6_has_custom_rules) {
> +		struct fib_lookup_arg arg = {
> +			.lookup_ptr = lookup,
> +			.flags = FIB_LOOKUP_NOREF,
> +		};
> +
> +		fib_rules_lookup(net->ipv6.fib6_rules_ops,
> +				 flowi6_to_flowi(fl6), flags, &arg);
> +
> +		if (arg.result)
> +			return arg.result;
> +	} else {
> +		struct rt6_info *rt;
> +
> +		rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, flags);
> +		if (rt != net->ipv6.ip6_null_entry && rt->dst.error != -EAGAIN)
> +			return &rt->dst;
> +		ip6_rt_put(rt);
> +		rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, flags);
> +		if (rt->dst.error != -EAGAIN)
> +			return &rt->dst;
> +		ip6_rt_put(rt);
> +	}
>  
>  	dst_hold(&net->ipv6.ip6_null_entry->dst);
>  	return &net->ipv6.ip6_null_entry->dst;

^ permalink raw reply

* [PATCH net-next] ibmvnic: Clean up resources on probe failure
From: Nathan Fontenot @ 2017-08-08 19:28 UTC (permalink / raw)
  To: netdev; +Cc: jallen, tlfalcon

Ensure that any resources allocated during probe are released if the
probe of the driver fails.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c |   26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5932160..24f536b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3827,31 +3827,35 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 
 	do {
 		rc = ibmvnic_init(adapter);
-		if (rc && rc != EAGAIN) {
-			free_netdev(netdev);
-			return rc;
-		}
+		if (rc && rc != EAGAIN)
+			goto ibmvnic_init_fail;
 	} while (rc == EAGAIN);
 
 	netdev->mtu = adapter->req_mtu - ETH_HLEN;
 
 	rc = device_create_file(&dev->dev, &dev_attr_failover);
-	if (rc) {
-		free_netdev(netdev);
-		return rc;
-	}
+	if (rc)
+		goto ibmvnic_init_fail;
 
 	rc = register_netdev(netdev);
 	if (rc) {
 		dev_err(&dev->dev, "failed to register netdev rc=%d\n", rc);
-		device_remove_file(&dev->dev, &dev_attr_failover);
-		free_netdev(netdev);
-		return rc;
+		goto ibmvnic_register_fail;
 	}
 	dev_info(&dev->dev, "ibmvnic registered\n");
 
 	adapter->state = VNIC_PROBED;
 	return 0;
+
+ibmvnic_register_fail:
+	device_remove_file(&dev->dev, &dev_attr_failover);
+	
+ibmvnic_init_fail:
+	release_sub_crqs(adapter);
+	release_crq_queue(adapter);
+	free_netdev(netdev);
+
+	return rc;
 }
 
 static int ibmvnic_remove(struct vio_dev *dev)

^ permalink raw reply related

* [PATCH v2 iproute2] lib: Dump ext-ack string by default
From: David Ahern @ 2017-08-08 14:30 UTC (permalink / raw)
  To: netdev, stephen; +Cc: David Ahern

In time, errfn can be implemented for link, route, etc commands to
give a much more detailed response (e.g., point to the attribute
that failed). Doing so is much more complicated to process the
message and convert attribute ids to names.

In any case the error string returned by the kernel should be dumped
to the user, so make that happen now.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
v2
- check that errmsg is non-null

 lib/libnetlink.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 145de2cb0ccf..063f5cd6c7b1 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -61,7 +61,6 @@ static int err_attr_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
-
 /* dump netlink extended ack error message */
 static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 {
@@ -72,9 +71,6 @@ static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 	const char *errmsg = NULL;
 	uint32_t off = 0;
 
-	if (!errfn)
-		return 0;
-
 	/* no TLVs, nothing to do here */
 	if (!(nlh->nlmsg_flags & NLM_F_ACK_TLVS))
 		return 0;
@@ -99,7 +95,16 @@ static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 			err_nlh = &err->msg;
 	}
 
-	return errfn(errmsg, off, err_nlh);
+	if (errfn)
+		return errfn(errmsg, off, err_nlh);
+
+	if (errmsg) {
+		fprintf(stderr, "Error: %s\n", errmsg);
+
+		return 1;
+	}
+
+	return 0;
 }
 #else
 #warning "libmnl required for error support"
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH net-next] openvswitch: add NSH support
From: Jiri Benc @ 2017-08-08 14:28 UTC (permalink / raw)
  To: Yi Yang; +Cc: netdev, dev, davem
In-Reply-To: <1502168380-106785-1-git-send-email-yi.y.yang@intel.com>

On Tue,  8 Aug 2017 12:59:40 +0800, Yi Yang wrote:
> +struct ovs_key_nsh {
> +	__u8 flags;
> +	__u8 mdtype;
> +	__u8 np;
> +	__u8 pad;
> +	__be32 path_hdr;
> +	__be32 c[4];

"c" is a very poor name. Please rename it to something that expresses
what this field contains.

Also, this looks like MD type 1 only. How are those fields going to work
with MD type 2? I don't think MD type 2 implementation is necessary in
this patch but I'd like to know how this is going to work - it's uAPI
and thus set in stone once this is merged. The uAPI needs to be
designed with future use in mind.

> +#define OVS_ENCAP_NSH_MAX_MD_LEN 16
> +/*
> + * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
> + * @flags: NSH header flags.
> + * @mdtype: NSH metadata type.
> + * @mdlen: Length of NSH metadata in bytes.
> + * @np: NSH next_protocol: Inner packet type.
> + * @path_hdr: NSH service path id and service index.
> + * @metadata: NSH metadata for MD type 1 or 2
> + */
> +struct ovs_action_encap_nsh {
> +	__u8 flags;
> +	__u8 mdtype;
> +	__u8 mdlen;
> +	__u8 np;
> +	__be32 path_hdr;
> +	__u8 metadata[OVS_ENCAP_NSH_MAX_MD_LEN];

This is wrong. The metadata size is set to a fixed size by this and
cannot be ever extended, or at least not easily. Netlink has attributes
for exactly these cases and that's what needs to be used here.

> @@ -835,6 +866,8 @@ enum ovs_action_attr {
>  	OVS_ACTION_ATTR_TRUNC,        /* u32 struct ovs_action_trunc. */
>  	OVS_ACTION_ATTR_PUSH_ETH,     /* struct ovs_action_push_eth. */
>  	OVS_ACTION_ATTR_POP_ETH,      /* No argument. */
> +	OVS_ACTION_ATTR_ENCAP_NSH,    /* struct ovs_action_encap_nsh. */
> +	OVS_ACTION_ATTR_DECAP_NSH,    /* No argument. */

Use "push" and "pop", not "encap" and "decap" to be consistent with the
naming in the rest of the file. We use encap and decap for tunnel
operations. This code does not use lwtunnels, thus push and pop is more
appropriate.

 Jiri

^ permalink raw reply

* Re:Re: [PATCH net] ppp: fix xmit recursion detection on ppp channels
From: Gao Feng @ 2017-08-08 14:21 UTC (permalink / raw)
  To: Guillaume Nault
  Cc: netdev, Liu Jianying, David Miller, linux-ppp, Paul Mackerras
In-Reply-To: <20170808135827.oszy6ikqi2gw5cs7@alphalink.fr>


At 2017-08-08 21:58:27, "Guillaume Nault" <g.nault@alphalink.fr> wrote:
>On Tue, Aug 08, 2017 at 09:16:33PM +0800, Gao Feng wrote:
>> At 2017-08-08 17:43:24, "Guillaume Nault" <g.nault@alphalink.fr> wrote:
>> >--- a/drivers/net/ppp/ppp_generic.c
>> >+++ b/drivers/net/ppp/ppp_generic.c
>> >@@ -1915,21 +1915,23 @@ static void __ppp_channel_push(struct channel *pch)
>> > 	spin_unlock(&pch->downl);
>> > 	/* see if there is anything from the attached unit to be sent */
>> > 	if (skb_queue_empty(&pch->file.xq)) {
>> >-		read_lock(&pch->upl);
>> > 		ppp = pch->ppp;
>> > 		if (ppp)
>> >-			ppp_xmit_process(ppp);
>> >-		read_unlock(&pch->upl);
>> >+			__ppp_xmit_process(ppp);
>> > 	}
>> > }
>> > 
>> > static void ppp_channel_push(struct channel *pch)
>> > {
>> >-	local_bh_disable();
>> >-
>> >-	__ppp_channel_push(pch);
>> >-
>> >-	local_bh_enable();
>> >+	read_lock_bh(&pch->upl);
>> >+	if (pch->ppp) {
>> >+		(*this_cpu_ptr(pch->ppp->xmit_recursion))++;
>> >+		__ppp_channel_push(pch);
>> >+		(*this_cpu_ptr(pch->ppp->xmit_recursion))--;
>> >+	} else {
>> >+		__ppp_channel_push(pch);
>> >+	}
>> >+	read_unlock_bh(&pch->upl);
>> 
>> If invoked read_lock_bh in ppp_channel_push, it would be unnecessary to invoke read_lock(&pch->upl)
>> in the __ppp_channel_push.
>> 
>But this patch does remove read_lock(&pch->upl) from
>__ppp_channel_push(). Or have I misunderstood your point?

Sorry, it's my fault.
I forgot your former changes when think about the updates in ppp_channel_push

Best Regards
Feng


^ permalink raw reply

* Re: l3mdev: Support for sockets bound to enslaved device
From: David Ahern @ 2017-08-08 14:19 UTC (permalink / raw)
  To: Ondrej Zajicek; +Cc: netdev
In-Reply-To: <20170808112958.ieexjwwgyp576kga@feanor.crfreenet.org>

On 8/8/17 5:29 AM, Ondrej Zajicek wrote:
> Hi
> 
> We noticed that TCP outgoing sockets that are bound to enslaved devices
> by SO_BINDTODEVICE fail to connect, while they work when bound to vrf
> device instead. We noticed similar behavior on ICMP (i.e. ping -I vrf0
> worked, while ping -I eth0 does not if eth0 is enslaved to vrf0). On
> the contrary, UDP outgoing sockets bound to enslaved devices worked
> as expected. We tested that on Linux 4.9.30-2 from Debian.
> 
> I found your patchset 'Support for sockets bound to enslaved device'
> ( http://www.spinics.net/lists/netdev/msg448040.html ), which seems
> to be related to the issue, but the description mentions services,
> i.e. listening sockets. Does the patchset (or some other one) fixes
> the issue also for outgoing sockets?
> 

Yes, it works for both directions.

^ permalink raw reply

* Re: qmi_wwan: Null pointer dereference when removing driver
From: Nathaniel Roach @ 2017-08-08 14:18 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev, Daniele Palmas
In-Reply-To: <87d1861h5x.fsf@miraculix.mork.no>

Yup, that patch works perfectly, and yes the device is a multi-tty device.

Cheers for everything!


On 08/08/17 18:35, Bjørn Mork wrote:
> Nathaniel Roach <nroach44@gmail.com> writes:
>
>> Unsure at which point was added, but issue not present in stock debian 4.11 kernel.
>>
>> Running on a Thinkpad X220 with coreboot.
>>
>> I'm building from upstream. When I attempt to remove the qmi_wwan module (which also happens pre-suspend) the rmmod process gets killed, and the following shows in dmesg:
>>
>> [   59.979791] usb 2-1.4: USB disconnect, device number 4
>> [   59.980102] qmi_wwan 2-1.4:1.6 wwp0s29u1u4i6: unregister 'qmi_wwan' usb-0000:00:1d.0-1.4, WWAN/QMI device
>> [   60.006821] BUG: unable to handle kernel NULL pointer dereference at 00000000000000e0
>> [   60.006879] IP: qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan]
>> [   60.006911] PGD 0
>> [   60.006911] P4D 0
>> [   60.006957] Oops: 0000 [#1] SMP
>> [   60.006978] Modules linked in: fuse(E) ccm(E) rfcomm(E) cmac(E) bnep(E) qmi_wwan(E) cdc_wdm(E) cdc_ether(E) usbnet(E) mii(E) btusb(E) btrtl(E) btbcm(E) btintel(E) bluetooth(E) joydev(E) xpad(E) ecdh_generic(E) ff_memless(E) binfmt_misc(E) snd_hda_codec_hdmi(E) snd_hda_codec_conexant(E) snd_hda_codec_generic(E) arc4(E) iTCO_wdt(E) iTCO_vendor_support(E) intel_rapl(E) x86_pkg_temp_thermal(E) kvm_intel(E) kvm(E) irqbypass(E) crct10dif_pclmul(E) crc32_pclmul(E) crc32c_intel(E) ghash_clmulni_intel(E) aesni_intel(E) iwlmvm(E) aes_x86_64(E) crypto_simd(E) mac80211(E) cryptd(E) glue_helper(E) snd_hda_intel(E) snd_hda_codec(E) iwlwifi(E) snd_hwdep(E) psmouse(E) snd_hda_core(E) snd_pcm(E) serio_raw(E) sdhci_pci(E) pcspkr(E) snd_timer(E) ehci_pci(E) e1000e(E) i2c_i801(E) ehci_hcd(E) snd(E) sg(E
 ) i915(E) lpc_ich(E)
>> [   60.007366]  ptp(E) usbcore(E) cfg80211(E) mfd_core(E) pps_core(E) shpchp(E) ac(E) battery(E) tpm_tis(E) tpm_tis_core(E) evdev(E) tpm(E) parport_pc(E) ppdev(E) lp(E) parport(E) ip_tables(E) x_tables(E) autofs4(E)
>> [   60.007474] CPU: 2 PID: 33 Comm: kworker/2:1 Tainted: G            E   4.12.3-nr44-normandy-r1500619820+ #1
>> [   60.007524] Hardware name: LENOVO 4291LR7/4291LR7, BIOS CBET4000 4.6-810-g50522254fb 07/21/2017
>> [   60.007580] Workqueue: usb_hub_wq hub_event [usbcore]
>> [   60.007609] task: ffff8c882b716040 task.stack: ffffb8e800d84000
>> [   60.007644] RIP: 0010:qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan]
>> [   60.007678] RSP: 0018:ffffb8e800d87b38 EFLAGS: 00010246
>> [   60.007711] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
>> [   60.007752] RDX: 0000000000000001 RSI: ffff8c8824f3f1d0 RDI: ffff8c8824ef6400
>> [   60.007792] RBP: ffff8c8824ef6400 R08: 0000000000000000 R09: 0000000000000000
>> [   60.007833] R10: ffffb8e800d87780 R11: 0000000000000011 R12: ffffffffc07ea0e8
>> [   60.007874] R13: ffff8c8824e2e000 R14: ffff8c8824e2e098 R15: 0000000000000000
>> [   60.007915] FS:  0000000000000000(0000) GS:ffff8c8835300000(0000) knlGS:0000000000000000
>> [   60.007960] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [   60.007994] CR2: 00000000000000e0 CR3: 0000000229ca5000 CR4: 00000000000406e0
>> [   60.008035] Call Trace:
>> [   60.008065]  ? usb_unbind_interface+0x71/0x270 [usbcore]
>> [   60.008101]  ? device_release_driver_internal+0x154/0x210
>> [   60.008135]  ? qmi_wwan_unbind+0x6d/0xc0 [qmi_wwan]
>> [   60.008168]  ? usbnet_disconnect+0x6c/0xf0 [usbnet]
>> [   60.008194]  ? qmi_wwan_disconnect+0x87/0xc0 [qmi_wwan]
>> [   60.008232]  ? usb_unbind_interface+0x71/0x270 [usbcore]
>> [   60.008264]  ? device_release_driver_internal+0x154/0x210
>> [   60.008296]  ? bus_remove_device+0xf5/0x160
>> [   60.008324]  ? device_del+0x1dc/0x310
>> [   60.008355]  ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
>> [   60.008393]  ? usb_disable_device+0x93/0x250 [usbcore]
>> [   60.008430]  ? usb_disconnect+0x90/0x260 [usbcore]
>> [   60.008468]  ? hub_event+0x1d9/0x14a0 [usbcore]
>> [   60.008500]  ? process_one_work+0x175/0x370
>> [   60.008528]  ? worker_thread+0x4a/0x380
>> [   60.008555]  ? kthread+0xfc/0x130
>> [   60.008579]  ? process_one_work+0x370/0x370
>> [   60.008606]  ? kthread_park+0x60/0x60
>> [   60.008631]  ? ret_from_fork+0x22/0x30
>> [   60.008656] Code: 66 0f 1f 44 00 00 66 66 66 66 90 55 48 89 fd 53 48 83 ec 10 48 8b 9f c8 00 00 00 65 48 8b 04 25 28 00 00 00 48 89 44 24 08 31 c0 <f6> 83 e0 00 00 00 02 74 51 e8 0d b3 2b cd 85 c0 74 67 48 8b bb
>> [   60.011925] RIP: qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan] RSP: ffffb8e800d87b38
>> [   60.013564] CR2: 00000000000000e0
>> [   60.022125] ---[ end trace e536b59f45bc0f25 ]---
>> [   60.025385] IPv6: ADDRCONF(NETDEV_UP): wlp2s0: link is not ready
>>
>> If I attempt a second rmmod, the process hangs. If I attempt it on 4.11.x it works as expected:
>>
>> [   16.897783] fuse init (API version 7.26)
>> [   68.073552] usbcore: deregistering interface driver qmi_wwan
>> [   68.075808] qmi_wwan 2-1.4:1.6 wwp0s29u1u4i6: unregister 'qmi_wwan' usb-0000:00:1d.0-1.4, WWAN/QMI device
>> [   72.431403] e1000e: enp0s25 NIC Link is Down
>>
>> So I'm pretty certain it's not coreboot causing the issue.
>
> Thanks a lot for the report!  Just one question: Does your modem have
> separate control and data interfaces? If so, then I believe the
> attached patch will fix the issue.  Are you able to test it?
>
> If the modem use the more common cobined interface model. then I need to
> investigate this further..
>
>
> Bjørn

^ permalink raw reply

* Re: [PATCH] net: phy: Use tab for indentation in Kconfig
From: Andrew Lunn @ 2017-08-08 14:10 UTC (permalink / raw)
  To: Michal Simek; +Cc: linux-kernel, monstr, netdev, Florian Fainelli
In-Reply-To: <abacdc7ec0585cfbeba240ca2780c6020cf7090a.1502184742.git.michal.simek@xilinx.com>

On Tue, Aug 08, 2017 at 11:32:25AM +0200, Michal Simek wrote:
> Using tabs instead of space for indentaion
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* FWD: [PATCH][next] net: phy: mdio-bcm-unimac: fix unsigned wrap-around when decrementing timeout
From: Andrew Lunn @ 2017-08-08 14:07 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: colin.king, netdev

----- Forwarded message from Colin King <colin.king@canonical.com> -----

Date: Tue,  8 Aug 2017 10:52:32 +0100
From: Colin King <colin.king@canonical.com>
To: Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>, netdev@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH][next] net: phy: mdio-bcm-unimac: fix unsigned wrap-around when decrementing timeout
X-Spam-Status: No, score=-7.4 required=4.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD,T_HEADER_FROM_DIFFERENT_DOMAINS autolearn=ham
	autolearn_force=no version=3.4.0

From: Colin Ian King <colin.king@canonical.com>

Change post-decrement compare to pre-decrement to avoid an
unsigned integer wrap-around on timeout. This leads to the following
!timeout check to never to be true so -ETIMEDOUT is never returned.

Detected by CoverityScan, CID#1452623 ("Logically dead code")

Fixes: 69a60b0579a4 ("net: phy: mdio-bcm-unimac: factor busy polling loop")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/phy/mdio-bcm-unimac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/mdio-bcm-unimac.c b/drivers/net/phy/mdio-bcm-unimac.c
index 89425ca48412..73c5267a11fd 100644
--- a/drivers/net/phy/mdio-bcm-unimac.c
+++ b/drivers/net/phy/mdio-bcm-unimac.c
@@ -71,7 +71,7 @@ static int unimac_mdio_poll(void *wait_func_data)
 			return 0;
 
 		usleep_range(1000, 2000);
-	} while (timeout--);
+	} while (--timeout);
 
 	if (!timeout)
 		return -ETIMEDOUT;
-- 
2.11.0


----- End forwarded message -----

Hi Florian

You might want to look at bcm_sf2_cfp_op() and bcm_sf2_cfp_rst().

Colin, do you have more of these patches? Have you done a tree wide
search?

       Andrew

^ permalink raw reply related


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