Netdev List
 help / color / mirror / Atom feed
* [PATCH V2 net] virtio-net: validate features during probe
From: Jason Wang @ 2014-11-19  7:21 UTC (permalink / raw)
  To: rusty, mst, virtualization, netdev, linux-kernel

This patch validates feature dependencies during probe and fail the probing
if a dependency is missed. This fixes the issues of hitting BUG()
when qemu fails to advertise features correctly. One example is booting
guest with ctrl_vq=off through qemu.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from V1:
- Drop VIRTIO_NET_F_*_UFO from the checklist, since it was disabled
---
 drivers/net/virtio_net.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..b16a761 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1673,6 +1673,93 @@ static const struct attribute_group virtio_net_mrg_rx_group = {
 };
 #endif
 
+static int virtnet_validate_features(struct virtio_device *dev,
+				     unsigned int *table,
+				     int table_size,
+				     unsigned int feature)
+{
+	int i;
+
+	if (!virtio_has_feature(dev, feature)) {
+		for (i = 0; i < table_size; i++) {
+			unsigned int f = table[i];
+
+			if (virtio_has_feature(dev, f)) {
+				dev_err(&dev->dev,
+					"buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x was not",
+					f, feature);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int virtnet_check_features(struct virtio_device *dev)
+{
+	unsigned int features_for_ctrl_vq[] = {
+		VIRTIO_NET_F_CTRL_RX,
+		VIRTIO_NET_F_CTRL_VLAN,
+		VIRTIO_NET_F_GUEST_ANNOUNCE,
+		VIRTIO_NET_F_MQ,
+		VIRTIO_NET_F_CTRL_MAC_ADDR
+	};
+	unsigned int features_for_guest_csum[] = {
+		VIRTIO_NET_F_GUEST_TSO4,
+		VIRTIO_NET_F_GUEST_TSO6,
+		VIRTIO_NET_F_GUEST_ECN,
+	};
+	unsigned int features_for_host_csum[] = {
+		VIRTIO_NET_F_HOST_TSO4,
+		VIRTIO_NET_F_HOST_TSO6,
+		VIRTIO_NET_F_HOST_ECN,
+	};
+	int err;
+
+	err = virtnet_validate_features(dev, features_for_ctrl_vq,
+					ARRAY_SIZE(features_for_ctrl_vq),
+					VIRTIO_NET_F_CTRL_VQ);
+	if (err)
+		return err;
+
+	err = virtnet_validate_features(dev, features_for_guest_csum,
+					ARRAY_SIZE(features_for_guest_csum),
+					VIRTIO_NET_F_GUEST_CSUM);
+	if (err)
+		return err;
+
+	err = virtnet_validate_features(dev, features_for_host_csum,
+					ARRAY_SIZE(features_for_host_csum),
+					VIRTIO_NET_F_CSUM);
+	if (err)
+		return err;
+
+	if (virtio_has_feature(dev, VIRTIO_NET_F_GUEST_ECN) &&
+	    (!virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO4) ||
+	     !virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO6))) {
+		dev_err(&dev->dev,
+			"buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+			VIRTIO_NET_F_GUEST_ECN,
+			VIRTIO_NET_F_GUEST_TSO4,
+			VIRTIO_NET_F_GUEST_TSO6);
+		return -EINVAL;
+	}
+
+	if (virtio_has_feature(dev, VIRTIO_NET_F_HOST_ECN) &&
+	    (!virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO4) ||
+	     !virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO6))) {
+		dev_err(&dev->dev,
+			"buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+			VIRTIO_NET_F_HOST_ECN,
+			VIRTIO_NET_F_HOST_TSO4,
+			VIRTIO_NET_F_HOST_TSO6);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int virtnet_probe(struct virtio_device *vdev)
 {
 	int i, err;
@@ -1680,6 +1767,10 @@ static int virtnet_probe(struct virtio_device *vdev)
 	struct virtnet_info *vi;
 	u16 max_queue_pairs;
 
+	err = virtnet_check_features(vdev);
+	if (err)
+		return -EINVAL;
+
 	/* Find if host supports multiqueue virtio_net device */
 	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
 				   struct virtio_net_config,
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: lucien xin @ 2014-11-19  7:10 UTC (permalink / raw)
  To: Cong Wang; +Cc: network dev
In-Reply-To: <CAHA+R7N7x5eMKFtyWpOcM_FGJ+9h-K+wLEwzpZya1kiztUo8RQ@mail.gmail.com>

>>
>
> Hmm, your fix is correct, but the description is wrong, we should just skip
> the unregister of fb tunnel device like other tunnels. Without ->dellink()
> the fb device is still removed from the global netdev list, this needs to skip,
> otherwise ip_vti0 will disappear from your system.

yes, you are right. the issue only exist in the fb tunnel device, I will change
the description.

^ permalink raw reply

* Re: [PATCH net] virtio-net: validate features during probe
From: Jason Wang @ 2014-11-19  7:07 UTC (permalink / raw)
  To: rusty, mst, virtualization, netdev, linux-kernel
In-Reply-To: <1416378939-28821-1-git-send-email-jasowang@redhat.com>

On 11/19/2014 02:35 PM, Jason Wang wrote:
> This patch validates feature dependencies during probe and fail the probing
> if a dependency is missed. This fixes the issues of hitting BUG()
> when qemu fails to advertise features correctly. One example is booting
> guest with ctrl_vq=off through qemu.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---

Just notice UFO has been disabled after
3d0ad09412ffe00c9afa201d01effdb6023d09b4 "drivers/net: Disable UFO
through virtio". Will post V2 and drop VIRTIO_NET_F_*_UFO from the
checklist.

^ permalink raw reply

* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Wengang @ 2014-11-19  7:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1416375565.14060.43.camel@edumazet-glaptop2.roam.corp.google.com>

Hi Eric,

于 2014年11月19日 13:39, Eric Dumazet 写道:
> On Wed, 2014-11-19 at 13:18 +0800, Wengang Wang wrote:
>> When last slave of a bonding master is removed, the bonding then does not work.
>> When packet_snd is called against with a master net_device, it accesses
>> header_ops. In case the header_ops is not valid any longer(say ipoib module
>> unloaded), it will then access an invalid memory address.
>> This patch try to fix this issue by clearing header_ops when last slave
>> detached.
>>
>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>> ---
>>   drivers/net/bonding/bond_main.c | 1 +
>>   1 file changed, 1 insertion(+)
> It seems you basically ignored my feedback.
>
>
>
> Some code is doing :
>
> [A] if (dev->header_ops) {
>      ...
> [B]    dev->header_ops->XXX()
>
>
>
> Nothing prevents you doing the clear after [A] , and before [B]
>

Yes, that's true. So the simplest way is move ipoib_header_ops to vmlinux.

thanks for review.
wengang

^ permalink raw reply

* Re: [BNX2] A Netdev Watchdog with kernel stable 3.4
From: Michael Chan @ 2014-11-19  6:58 UTC (permalink / raw)
  To: Rui Xiang, sony.chacko; +Cc: netdev
In-Reply-To: <546C388B.2050603@huawei.com>

Copying the current maintainer Sony.  The PCI command register looks
strange.  Please see below.

On Wed, 2014-11-19 at 14:28 +0800, Rui Xiang wrote: 
> ping...
> 
> On 2014/11/17 20:42, Rui Xiang wrote:
> > Hi Michael,
> > 
> > On a system that was running stable 3.4.87, I got the below stack.
> > That was a NETDEV WATCHDOG. And we could also see watchdog timeouts with the 
> > BNX2. (After the stack, an oops occurred while running ifconfig. I think it 
> > would be related to this timeout.)
> > 
> > Otherwises, the bnx2_dump_state and bnx2_dump_mcp_state have printed the states. 
> > Through these states info, can we got the real situation of NIC1.
> > Or can we see what resulted the WATCHDOG, a bnx2 device fault or other reasons.
> > 
> > Thanks.
> > 
> > 
> > *The stack*:
> > 
> >  WARNING: at /usr/src/packages/BUILD/kernel-default-3.4.87/linux-3.4/net/sched/sch_generic.c:256 dev_watchdog+0x256/0x260()
> >  NETDEV WATCHDOG: NIC1 (bnx2): transmit queue 3 timed out
> >  Modules linked in: smb3_failover(O) smb2(O) smb(O) smb_manager(O) nfs(O) nfs_acl(O) nfsd(O) lockd(O) nal(O) auth_rpcgss(O) 
> > scsi_dh_hp_sw scsi_dh_alua scsi_dh_emc scsi_dh_rdac scsi_dh scsi_mod [last unloaded: ipmi_msghandler]
> >  Pid: 0, comm: swapper/0 Tainted: P        W  O 3.4.87-default #1
> >  Call Trace:
> >   <IRQ>  [<ffffffff8103fcea>] warn_slowpath_common+0x7a/0xb0
> >   [<ffffffff8103fdc1>] warn_slowpath_fmt+0x41/0x50
> >   [<ffffffff81047749>] ? raise_softirq_irqoff+0x9/0x30
> >   [<ffffffff813ae0f6>] dev_watchdog+0x256/0x260
> >   [<ffffffff813adea0>] ? dev_deactivate_queue.constprop.30+0x70/0x70
> >   [<ffffffff8104edc7>] run_timer_softirq+0x147/0x340
> >   [<ffffffff810470d8>] __do_softirq+0xc8/0x1e0
> >   [<ffffffff8109250f>] ? tick_program_event+0x1f/0x30
> >   [<ffffffff81460a6c>] call_softirq+0x1c/0x30
> >   [<ffffffff8100417d>] do_softirq+0x9d/0xd0
> >   [<ffffffff810474a5>] irq_exit+0xb5/0xc0
> >   [<ffffffff81021b49>] smp_apic_timer_interrupt+0x69/0xa0
> >   [<ffffffff8146006f>] apic_timer_interrupt+0x6f/0x80
> >   <EOI>  [<ffffffff81457bdd>] ? retint_restore_args+0x13/0x13
> >   [<ffffffff81360149>] ? poll_idle+0x49/0x90
> >   [<ffffffff8136011f>] ? poll_idle+0x1f/0x90
> >   [<ffffffff8135fcc9>] cpuidle_enter+0x19/0x20
> >   [<ffffffff813602f2>] cpuidle_idle_call+0xa2/0x250
> >   [<ffffffff8100b08f>] cpu_idle+0x6f/0xe0
> >   [<ffffffff81915960>] ? rawsock_init+0x12/0x12
> >   [<ffffffff814331c9>] rest_init+0x6d/0x74
> >   [<ffffffff818d3be5>] start_kernel+0x3a2/0x3af
> >   [<ffffffff818d3642>] ? repair_env_string+0x5e/0x5e
> >   [<ffffffff818d332a>] x86_64_start_reservations+0x131/0x135
> >   [<ffffffff818d342e>] x86_64_start_kernel+0x100/0x10f
> >  ---[ end trace 497e24e681e0c02d ]---
> >  bnx2 0000:05:00.1: NIC1: DEBUG: intr_sem[0] PCI_CMD[00100002]

The memory bit in PCI_CMD is set, but the bus master bit is not set.
DMA won't work if the bus master bit is not set.  What was happening
before the timeout?  Was it working fine for a while and it suddenly
stopped?

> >  bnx2 0000:05:00.1: NIC1: DEBUG: PCI_PM[19002008] PCI_MISC_CFG[92000088]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: EMAC_TX_STATUS[00000008] EMAC_RX_STATUS[00000000]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: RPM_MGMT_PKT_CTRL[40000088]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: HC_STATS_INTERRUPT_STATUS[01ff0000]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: PBA[00000000]
> >  bnx2 0000:05:00.1: NIC1: <--- start MCP states dump --->
> >  bnx2 0000:05:00.1: NIC1: DEBUG: MCP_STATE_P0[0003e10e] MCP_STATE_P1[0003e10e]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: MCP mode[0000b800] state[80008000] evt_mask[00000500]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: pc[08008f60] pc[0800d21c] instr[00051080]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: shmem states:
> >  bnx2 0000:05:00.1: NIC1: DEBUG: drv_mb[01030003] fw_mb[00000003] link_status[0000006f] drv_pulse_mb[0000073d]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: dev_info_signature[44564907] reset_type[01005254] condition[0003e10e]
> >  bnx2 0000:05:00.1: NIC1: DEBUG: 000003cc: 00000000 00000000 00000000 00000000
> >  bnx2 0000:05:00.1: NIC1: DEBUG: 000003dc: 00000000 00000000 00000000 00000000
> >  bnx2 0000:05:00.1: NIC1: DEBUG: 000003ec: 00000000 00000000 00000000 00000000
> >  bnx2 0000:05:00.1: NIC1: DEBUG: 0x3fc[00000000]
> >  bnx2 0000:05:00.1: NIC1: <--- end MCP states dump --->
> > 
> 
> 

^ permalink raw reply

* [PATCH net] tcp: fix connect() invalid -EADDRNOTAVAIL error
From: Jon Maxwell @ 2014-11-19  6:37 UTC (permalink / raw)
  To: davem
  Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel, jmaxwell,
	Jon Maxwell

The connect() routine returns -EADDRNOTAVAIL without doing a 4 
tuple check when the hash buckets were previously allocated by 
bind() and all local ports are used.

The bind() routine creates the local port hash buckets in 
inet_csk_get_port(). Depending on the socket options it sets 
tb->fastreuse and tb->fastreuseport to 0 or 1 in the bucket.

However the __inet_hash_connect() routine initializes the hash 
buckets differently and sets these to -1. The end result is 
that connect() calling into __inet_hash_connect() will 
subsequently ignore the check_established() routine if, here

__inet_hash_connect()
.
.
if (tb->fastreuse >= 0 ||↩
    tb->fastreuseport >= 0)↩
    goto next_port;

and cycle through all local ports until it returns -EADDRNOTAVAIL. 
The 4 tuple check is in check_established() so connect() can fail 
unnecessarily.

Prerequisites for this to happen:
1) The local tcp port range must be exhausted.
2) A process must have called bind() followed by connect() for all 
local ports.
3) A different process calls connect() only which returns -EADDRNOTAVAIL. 
4) The system more than 1 interface configured.

If a system has 2 IP Addresses and all local tcp ports are in use
for connection from IP Address (1). Connecting to the same ports 
via IP Address (2) should work based on the 4 tuple rule. But it 
fails under this condition. 

To fix this make __inet_hash_connect() honour inet_csk_get_port()'s
tb->fastreuse* variables.

Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
 net/ipv4/inet_hashtables.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 9111a4e..b39e89e 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -513,8 +513,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 			inet_bind_bucket_for_each(tb, &head->chain) {
 				if (net_eq(ib_net(tb), net) &&
 				    tb->port == port) {
-					if (tb->fastreuse >= 0 ||
-					    tb->fastreuseport >= 0)
+					if (tb->fastreuse > 0 ||
+					    tb->fastreuseport > 0)
 						goto next_port;
 					WARN_ON(hlist_empty(&tb->owners));
 					if (!check_established(death_row, sk,
@@ -530,8 +530,6 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 				spin_unlock(&head->lock);
 				break;
 			}
-			tb->fastreuse = -1;
-			tb->fastreuseport = -1;
 			goto ok;
 
 		next_port:
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.
From: Richard Cochran @ 2014-11-19  6:37 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, bruce.w.allan, Jacob Keller, Jeff Kirsher,
	John Ronciak, Matthew Vick
In-Reply-To: <e5b4ef7c56119e2d86e42f5680b1b82c9ec1a38e.1416265321.git.richardcochran@gmail.com>

On Tue, Nov 18, 2014 at 12:06:24AM +0100, Richard Cochran wrote:
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -5386,8 +5386,14 @@ void igb_update_stats(struct igb_adapter *adapter,
>  static void igb_tsync_interrupt(struct igb_adapter *adapter)
>  {
>  	struct e1000_hw *hw = &adapter->hw;
> +	struct ptp_clock_event event;
>  	u32 tsicr = rd32(E1000_TSICR);
>  
> +	if (tsicr & TSINTR_SYS_WRAP) {
> +		event.type = PTP_CLOCK_PPS;
> +		ptp_clock_event(adapter->ptp_clock, &event);

This causes a BUG for the 82580. When you enable E1000_TSICR_TXTS, it
seems that TSINTR_SYS_WRAP is also automatically enabled. Because the
82580 variant has no pps device enabled, the driver then dereferences
a null pointer.

So we need to make the call to ptp_clock_event() conditional based on
whether pps is enabled. I'll fix this in V2.

> +	}
> +

Thanks,
Richard

^ permalink raw reply

* [PATCH net] virtio-net: validate features during probe
From: Jason Wang @ 2014-11-19  6:35 UTC (permalink / raw)
  To: rusty, mst, virtualization, netdev, linux-kernel

This patch validates feature dependencies during probe and fail the probing
if a dependency is missed. This fixes the issues of hitting BUG()
when qemu fails to advertise features correctly. One example is booting
guest with ctrl_vq=off through qemu.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..4a0ad46 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1673,6 +1673,95 @@ static const struct attribute_group virtio_net_mrg_rx_group = {
 };
 #endif
 
+static int virtnet_validate_features(struct virtio_device *dev,
+				     unsigned int *table,
+				     int table_size,
+				     unsigned int feature)
+{
+	int i;
+
+	if (!virtio_has_feature(dev, feature)) {
+		for (i = 0; i < table_size; i++) {
+			unsigned int f = table[i];
+
+			if (virtio_has_feature(dev, f)) {
+				dev_err(&dev->dev,
+					"buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x was not",
+					f, feature);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int virtnet_check_features(struct virtio_device *dev)
+{
+	unsigned int features_for_ctrl_vq[] = {
+		VIRTIO_NET_F_CTRL_RX,
+		VIRTIO_NET_F_CTRL_VLAN,
+		VIRTIO_NET_F_GUEST_ANNOUNCE,
+		VIRTIO_NET_F_MQ,
+		VIRTIO_NET_F_CTRL_MAC_ADDR
+	};
+	unsigned int features_for_guest_csum[] = {
+		VIRTIO_NET_F_GUEST_TSO4,
+		VIRTIO_NET_F_GUEST_TSO6,
+		VIRTIO_NET_F_GUEST_ECN,
+		VIRTIO_NET_F_GUEST_UFO,
+	};
+	unsigned int features_for_host_csum[] = {
+		VIRTIO_NET_F_HOST_TSO4,
+		VIRTIO_NET_F_HOST_TSO6,
+		VIRTIO_NET_F_HOST_ECN,
+		VIRTIO_NET_F_HOST_UFO,
+	};
+	int err;
+
+	err = virtnet_validate_features(dev, features_for_ctrl_vq,
+					ARRAY_SIZE(features_for_ctrl_vq),
+					VIRTIO_NET_F_CTRL_VQ);
+	if (err)
+		return err;
+
+	err = virtnet_validate_features(dev, features_for_guest_csum,
+					ARRAY_SIZE(features_for_guest_csum),
+					VIRTIO_NET_F_GUEST_CSUM);
+	if (err)
+		return err;
+
+	err = virtnet_validate_features(dev, features_for_host_csum,
+					ARRAY_SIZE(features_for_host_csum),
+					VIRTIO_NET_F_CSUM);
+	if (err)
+		return err;
+
+	if (virtio_has_feature(dev, VIRTIO_NET_F_GUEST_ECN) &&
+	    (!virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO4) ||
+	     !virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO6))) {
+		dev_err(&dev->dev,
+			"buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+			VIRTIO_NET_F_GUEST_ECN,
+			VIRTIO_NET_F_GUEST_TSO4,
+			VIRTIO_NET_F_GUEST_TSO6);
+		return -EINVAL;
+	}
+
+	if (virtio_has_feature(dev, VIRTIO_NET_F_HOST_ECN) &&
+	    (!virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO4) ||
+	     !virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO6))) {
+		dev_err(&dev->dev,
+			"buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+			VIRTIO_NET_F_HOST_ECN,
+			VIRTIO_NET_F_HOST_TSO4,
+			VIRTIO_NET_F_HOST_TSO6);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int virtnet_probe(struct virtio_device *vdev)
 {
 	int i, err;
@@ -1680,6 +1769,10 @@ static int virtnet_probe(struct virtio_device *vdev)
 	struct virtnet_info *vi;
 	u16 max_queue_pairs;
 
+	err = virtnet_check_features(vdev);
+	if (err)
+		return -EINVAL;
+
 	/* Find if host supports multiqueue virtio_net device */
 	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
 				   struct virtio_net_config,
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net-next 0/4] igb: auxiliary PHC functions for the i210.
From: Richard Cochran @ 2014-11-19  6:31 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: netdev, David Miller, bruce.w.allan, Jacob Keller, John Ronciak,
	Matthew Vick
In-Reply-To: <1416266118.2983.3.camel@jtkirshe-mobl.jf.intel.com>

On Mon, Nov 17, 2014 at 03:15:18PM -0800, Jeff Kirsher wrote:
> Thanks Richard, I have added your series of igb patches to my queue.

Jeff, please hold off on these. I found a bug in the PPS code. I will
re-submit with a fix.

Thanks,
Richard

^ permalink raw reply

* Re: [BNX2] A Netdev Watchdog with kernel stable 3.4
From: Rui Xiang @ 2014-11-19  6:28 UTC (permalink / raw)
  To: Michael Chan; +Cc: netdev
In-Reply-To: <5469ED24.7010008@huawei.com>

ping...

On 2014/11/17 20:42, Rui Xiang wrote:
> Hi Michael,
> 
> On a system that was running stable 3.4.87, I got the below stack.
> That was a NETDEV WATCHDOG. And we could also see watchdog timeouts with the 
> BNX2. (After the stack, an oops occurred while running ifconfig. I think it 
> would be related to this timeout.)
> 
> Otherwises, the bnx2_dump_state and bnx2_dump_mcp_state have printed the states. 
> Through these states info, can we got the real situation of NIC1.
> Or can we see what resulted the WATCHDOG, a bnx2 device fault or other reasons.
> 
> Thanks.
> 
> 
> *The stack*:
> 
>  WARNING: at /usr/src/packages/BUILD/kernel-default-3.4.87/linux-3.4/net/sched/sch_generic.c:256 dev_watchdog+0x256/0x260()
>  NETDEV WATCHDOG: NIC1 (bnx2): transmit queue 3 timed out
>  Modules linked in: smb3_failover(O) smb2(O) smb(O) smb_manager(O) nfs(O) nfs_acl(O) nfsd(O) lockd(O) nal(O) auth_rpcgss(O) 
> scsi_dh_hp_sw scsi_dh_alua scsi_dh_emc scsi_dh_rdac scsi_dh scsi_mod [last unloaded: ipmi_msghandler]
>  Pid: 0, comm: swapper/0 Tainted: P        W  O 3.4.87-default #1
>  Call Trace:
>   <IRQ>  [<ffffffff8103fcea>] warn_slowpath_common+0x7a/0xb0
>   [<ffffffff8103fdc1>] warn_slowpath_fmt+0x41/0x50
>   [<ffffffff81047749>] ? raise_softirq_irqoff+0x9/0x30
>   [<ffffffff813ae0f6>] dev_watchdog+0x256/0x260
>   [<ffffffff813adea0>] ? dev_deactivate_queue.constprop.30+0x70/0x70
>   [<ffffffff8104edc7>] run_timer_softirq+0x147/0x340
>   [<ffffffff810470d8>] __do_softirq+0xc8/0x1e0
>   [<ffffffff8109250f>] ? tick_program_event+0x1f/0x30
>   [<ffffffff81460a6c>] call_softirq+0x1c/0x30
>   [<ffffffff8100417d>] do_softirq+0x9d/0xd0
>   [<ffffffff810474a5>] irq_exit+0xb5/0xc0
>   [<ffffffff81021b49>] smp_apic_timer_interrupt+0x69/0xa0
>   [<ffffffff8146006f>] apic_timer_interrupt+0x6f/0x80
>   <EOI>  [<ffffffff81457bdd>] ? retint_restore_args+0x13/0x13
>   [<ffffffff81360149>] ? poll_idle+0x49/0x90
>   [<ffffffff8136011f>] ? poll_idle+0x1f/0x90
>   [<ffffffff8135fcc9>] cpuidle_enter+0x19/0x20
>   [<ffffffff813602f2>] cpuidle_idle_call+0xa2/0x250
>   [<ffffffff8100b08f>] cpu_idle+0x6f/0xe0
>   [<ffffffff81915960>] ? rawsock_init+0x12/0x12
>   [<ffffffff814331c9>] rest_init+0x6d/0x74
>   [<ffffffff818d3be5>] start_kernel+0x3a2/0x3af
>   [<ffffffff818d3642>] ? repair_env_string+0x5e/0x5e
>   [<ffffffff818d332a>] x86_64_start_reservations+0x131/0x135
>   [<ffffffff818d342e>] x86_64_start_kernel+0x100/0x10f
>  ---[ end trace 497e24e681e0c02d ]---
>  bnx2 0000:05:00.1: NIC1: DEBUG: intr_sem[0] PCI_CMD[00100002]
>  bnx2 0000:05:00.1: NIC1: DEBUG: PCI_PM[19002008] PCI_MISC_CFG[92000088]
>  bnx2 0000:05:00.1: NIC1: DEBUG: EMAC_TX_STATUS[00000008] EMAC_RX_STATUS[00000000]
>  bnx2 0000:05:00.1: NIC1: DEBUG: RPM_MGMT_PKT_CTRL[40000088]
>  bnx2 0000:05:00.1: NIC1: DEBUG: HC_STATS_INTERRUPT_STATUS[01ff0000]
>  bnx2 0000:05:00.1: NIC1: DEBUG: PBA[00000000]
>  bnx2 0000:05:00.1: NIC1: <--- start MCP states dump --->
>  bnx2 0000:05:00.1: NIC1: DEBUG: MCP_STATE_P0[0003e10e] MCP_STATE_P1[0003e10e]
>  bnx2 0000:05:00.1: NIC1: DEBUG: MCP mode[0000b800] state[80008000] evt_mask[00000500]
>  bnx2 0000:05:00.1: NIC1: DEBUG: pc[08008f60] pc[0800d21c] instr[00051080]
>  bnx2 0000:05:00.1: NIC1: DEBUG: shmem states:
>  bnx2 0000:05:00.1: NIC1: DEBUG: drv_mb[01030003] fw_mb[00000003] link_status[0000006f] drv_pulse_mb[0000073d]
>  bnx2 0000:05:00.1: NIC1: DEBUG: dev_info_signature[44564907] reset_type[01005254] condition[0003e10e]
>  bnx2 0000:05:00.1: NIC1: DEBUG: 000003cc: 00000000 00000000 00000000 00000000
>  bnx2 0000:05:00.1: NIC1: DEBUG: 000003dc: 00000000 00000000 00000000 00000000
>  bnx2 0000:05:00.1: NIC1: DEBUG: 000003ec: 00000000 00000000 00000000 00000000
>  bnx2 0000:05:00.1: NIC1: DEBUG: 0x3fc[00000000]
>  bnx2 0000:05:00.1: NIC1: <--- end MCP states dump --->
> 

^ permalink raw reply

* Re: [PATCH net] bonding: fix curr_active_slave/carrier with loadbalance arp monitoring
From: Ding Tianhong @ 2014-11-19  6:20 UTC (permalink / raw)
  To: Andy Gospodarek, Veaceslav Falico
  Cc: Nikolay Aleksandrov, netdev, davem, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <20141118152847.GB2002@gospo.home.greyhouse.net>

On 2014/11/18 23:28, Andy Gospodarek wrote:
> On Tue, Nov 18, 2014 at 03:37:27PM +0100, Veaceslav Falico wrote:
>> On Tue, Nov 18, 2014 at 03:14:44PM +0100, Nikolay Aleksandrov wrote:
>>> Since commit 6fde8f037e60 ("bonding: fix locking in
>>> bond_loadbalance_arp_mon()") we can have a stale bond carrier state and
>>> stale curr_active_slave when using arp monitoring in loadbalance modes. The
>>> reason is that in bond_loadbalance_arp_mon() we can't have
>>> do_failover == true but slave_state_changed == false, whenever do_failover
>>> is true then slave_state_changed is also true. Then the following piece
>> >from bond_loadbalance_arp_mon():
>>>               if (slave_state_changed) {
>>>                       bond_slave_state_change(bond);
>>>                       if (BOND_MODE(bond) == BOND_MODE_XOR)
>>>                               bond_update_slave_arr(bond, NULL);
>>>               } else if (do_failover) {
>>
>> Ouch, must have been a big PITA to track :).
> 
> Agreed!
> 
>>
>>>                       block_netpoll_tx();
>>>                       bond_select_active_slave(bond);
>>>                       unblock_netpoll_tx();
>>>               }
>>>
>>> will execute only the first branch, always and regardless of do_failover.
>>> Since these two events aren't related in such way, we need to decouple and
>>> consider them separately.
>>>
>>> For example this issue could lead to the following result:
>>> Bonding Mode: load balancing (round-robin)
>>> *MII Status: down*
>>> MII Polling Interval (ms): 0
>>> Up Delay (ms): 0
>>> Down Delay (ms): 0
>>> ARP Polling Interval (ms): 100
>>> ARP IP target/s (n.n.n.n form): 192.168.9.2
>>>
>>> Slave Interface: ens12
>>> *MII Status: up*
>>> Speed: 10000 Mbps
>>> Duplex: full
>>> Link Failure Count: 2
>>> Permanent HW addr: 00:0f:53:01:42:2c
>>> Slave queue ID: 0
>>>
>>> Slave Interface: eth1
>>> *MII Status: up*
>>> Speed: Unknown
>>> Duplex: Unknown
>>> Link Failure Count: 70
>>> Permanent HW addr: 52:54:00:2f:0f:8e
>>> Slave queue ID: 0
>>>
>>> Since some interfaces are up, then the status of the bond should also be
>>> up, but it will never change unless something invokes bond_set_carrier()
>>> (i.e. enslave, bond_select_active_slave etc). Now, if I force the
>>> calling of bond_select_active_slave via for example changing
>>> primary_reselect (it can change in any mode), then the MII status goes to
>>> "up" because it calls bond_select_active_slave() which should've been done
>> >from bond_loadbalance_arp_mon() itself.
>>>
>>> CC: Veaceslav Falico <vfalico@gmail.com>
>>
>> Acked-by: Veaceslav Falico <vfalico@gmail.com>
> 
> Acked-by: Andy Gospodarek <gospo@cumulusnetworks.com>
> 
Acked-by: Ding Tianhong <dingtianhong@huawei.com>

>>
>>> CC: Jay Vosburgh <j.vosburgh@gmail.com>
>>> CC: Andy Gospodarek <andy@greyhouse.net>
>>> CC: Ding Tianhong <dingtianhong@huawei.com>
>>>
>>> Fixes: 6fde8f037e60 ("bonding: fix locking in bond_loadbalance_arp_mon()")
>>> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>>> ---
>>> Note: I left the parent if() the same even though we can shorten it. I think
>>>     it's better this way since it shows that any of the two events can cause
>>>     it to enter even though currently we can't have do_failover without
>>>     slave_state_changed, that may also change in the future.
>>>
>>> drivers/net/bonding/bond_main.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> index c9ac06cfe6b7..a5115fb7cf33 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -2471,7 +2471,8 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
>>> 			bond_slave_state_change(bond);
>>> 			if (BOND_MODE(bond) == BOND_MODE_XOR)
>>> 				bond_update_slave_arr(bond, NULL);
>>> -		} else if (do_failover) {
>>> +		}
>>> +		if (do_failover) {
>>> 			block_netpoll_tx();
>>> 			bond_select_active_slave(bond);
>>> 			unblock_netpoll_tx();
>>> -- 
>>> 1.9.3
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

^ permalink raw reply

* Re: [PATCH v2 net-next] tcp: make connect() mem charging friendly
From: Yuchung Cheng @ 2014-11-19  6:10 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Denys Fedoryshchenko, David Miller, netdev, Neal Cardwell
In-Reply-To: <1416294380.14060.6.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Nov 18, 2014 at 3:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> While working on sk_forward_alloc problems reported by Denys
> Fedoryshchenko, we found that tcp connect() (and fastopen) do not call
> sk_wmem_schedule() for SYN packet (and/or SYN/DATA packet), so
> sk_forward_alloc is negative while connect is in progress.
>
> We can fix this by calling regular sk_stream_alloc_skb() both for the
> SYN packet (in tcp_connect()) and the syn_data packet in
> tcp_send_syn_data()
>
> Then, tcp_send_syn_data() can avoid copying syn_data as we simply
> can manipulate syn_data->cb[] to remove SYN flag (and increment seq)
>
> Instead of open coding memcpy_fromiovecend(), simply use this helper.
>
> This leaves in socket write queue clean fast clone skbs.
>
> This was tested against our fastopen packetdrill tests.
>
> Reported-by: Denys Fedoryshchenko <nuclearcat@nuclearcat.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>

Thanks! this simplifies the code a lot.

> ---
> v2: added a kfree_skb(syn_data) if memcpy_fromiovecend() fails,
>     as spotted by Yuchung.
>
>  net/ipv4/tcp_output.c |   68 ++++++++++++++++------------------------
>  1 file changed, 28 insertions(+), 40 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index eb73a1dccf56b823a45c0ca034e40dc50fc48068..f5bd4bd3f7e669b3fd48a843d55e7313a30a3409 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -3011,9 +3011,9 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>         struct tcp_fastopen_request *fo = tp->fastopen_req;
> -       int syn_loss = 0, space, i, err = 0, iovlen = fo->data->msg_iovlen;
> -       struct sk_buff *syn_data = NULL, *data;
> +       int syn_loss = 0, space, err = 0;
>         unsigned long last_syn_loss = 0;
> +       struct sk_buff *syn_data;
>
>         tp->rx_opt.mss_clamp = tp->advmss;  /* If MSS is not cached */
>         tcp_fastopen_cache_get(sk, &tp->rx_opt.mss_clamp, &fo->cookie,
> @@ -3044,48 +3044,40 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
>         /* limit to order-0 allocations */
>         space = min_t(size_t, space, SKB_MAX_HEAD(MAX_TCP_HEADER));
>
> -       syn_data = skb_copy_expand(syn, MAX_TCP_HEADER, space,
> -                                  sk->sk_allocation);
> -       if (syn_data == NULL)
> +       syn_data = sk_stream_alloc_skb(sk, space, sk->sk_allocation);
> +       if (!syn_data)
>                 goto fallback;
> +       syn_data->ip_summed = CHECKSUM_PARTIAL;
> +       memcpy(syn_data->cb, syn->cb, sizeof(syn->cb));
> +       if (unlikely(memcpy_fromiovecend(skb_put(syn_data, space),
> +                                        fo->data->msg_iov, 0, space))) {
> +               kfree_skb(syn_data);
> +               goto fallback;
> +       }
>
> -       for (i = 0; i < iovlen && syn_data->len < space; ++i) {
> -               struct iovec *iov = &fo->data->msg_iov[i];
> -               unsigned char __user *from = iov->iov_base;
> -               int len = iov->iov_len;
> +       /* No more data pending in inet_wait_for_connect() */
> +       if (space == fo->size)
> +               fo->data = NULL;
> +       fo->copied = space;
>
> -               if (syn_data->len + len > space)
> -                       len = space - syn_data->len;
> -               else if (i + 1 == iovlen)
> -                       /* No more data pending in inet_wait_for_connect() */
> -                       fo->data = NULL;
> +       tcp_connect_queue_skb(sk, syn_data);
>
> -               if (skb_add_data(syn_data, from, len))
> -                       goto fallback;
> -       }
> +       err = tcp_transmit_skb(sk, syn_data, 1, sk->sk_allocation);
>
> -       /* Queue a data-only packet after the regular SYN for retransmission */
> -       data = pskb_copy(syn_data, sk->sk_allocation);
> -       if (data == NULL)
> -               goto fallback;
> -       TCP_SKB_CB(data)->seq++;
> -       TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN;
> -       TCP_SKB_CB(data)->tcp_flags = (TCPHDR_ACK|TCPHDR_PSH);
> -       tcp_connect_queue_skb(sk, data);
> -       fo->copied = data->len;
> -
> -       /* syn_data is about to be sent, we need to take current time stamps
> -        * for the packets that are in write queue : SYN packet and DATA
> -        */
> -       skb_mstamp_get(&syn->skb_mstamp);
> -       data->skb_mstamp = syn->skb_mstamp;
> +       syn->skb_mstamp = syn_data->skb_mstamp;
>
> -       if (tcp_transmit_skb(sk, syn_data, 0, sk->sk_allocation) == 0) {
> +       /* Now full SYN+DATA was cloned and sent (or not),
> +        * remove the SYN from the original skb (syn_data)
> +        * we keep in write queue in case of a retransmit, as we
> +        * also have the SYN packet (with no data) in the same queue.
> +        */
> +       TCP_SKB_CB(syn_data)->seq++;
> +       TCP_SKB_CB(syn_data)->tcp_flags = TCPHDR_ACK | TCPHDR_PSH;
> +       if (!err) {
>                 tp->syn_data = (fo->copied > 0);
>                 NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPORIGDATASENT);
>                 goto done;
>         }
> -       syn_data = NULL;
>
>  fallback:
>         /* Send a regular SYN with Fast Open cookie request option */
> @@ -3094,7 +3086,6 @@ fallback:
>         err = tcp_transmit_skb(sk, syn, 1, sk->sk_allocation);
>         if (err)
>                 tp->syn_fastopen = 0;
> -       kfree_skb(syn_data);
>  done:
>         fo->cookie.len = -1;  /* Exclude Fast Open option for SYN retries */
>         return err;
> @@ -3114,13 +3105,10 @@ int tcp_connect(struct sock *sk)
>                 return 0;
>         }
>
> -       buff = alloc_skb_fclone(MAX_TCP_HEADER + 15, sk->sk_allocation);
> -       if (unlikely(buff == NULL))
> +       buff = sk_stream_alloc_skb(sk, 0, sk->sk_allocation);
> +       if (unlikely(!buff))
>                 return -ENOBUFS;
>
> -       /* Reserve space for headers. */
> -       skb_reserve(buff, MAX_TCP_HEADER);
> -
>         tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
>         tp->retrans_stamp = tcp_time_stamp;
>         tcp_connect_queue_skb(sk, buff);
>
>

^ permalink raw reply

* Re: [PATCH net] openvswitch: Fix mask generation for IPv6 labels.
From: Pravin Shelar @ 2014-11-19  6:09 UTC (permalink / raw)
  To: Joe Stringer; +Cc: netdev, LKML, dev@openvswitch.org
In-Reply-To: <1416336857-61405-1-git-send-email-joestringer@nicira.com>

On Tue, Nov 18, 2014 at 10:54 AM, Joe Stringer <joestringer@nicira.com> wrote:
> When userspace doesn't provide a mask, OVS datapath generates a fully
> unwildcarded mask for the flow. This is done by taking a copy of the
> flow key, then iterating across its attributes, setting all values to
> 0xff. This works for most attributes, as the length of the netlink
> attribute typically matches the length of the value. However, IPv6
> labels only use the lower 20 bits of the field. This patch makes a
> special case to handle this.
>
> This fixes the following error seen when installing IPv6 flows without a mask:
>
> openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
>
We should allow exact match mask here rather than generating
wildcarded mask. So that ovs can catch invalid ipv6.label.


> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
>  net/openvswitch/flow_netlink.c |   22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index fa4ec2e..7a5b28f 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -825,7 +825,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
>         return 0;
>  }
>
> -static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
> +static void mask_set_nlattr(struct nlattr *attr)
>  {
>         struct nlattr *nla;
>         int rem;
> @@ -835,16 +835,18 @@ static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
>                 /* We assume that ovs_key_lens[type] == -1 means that type is a
>                  * nested attribute
>                  */
> -               if (is_attr_mask_key && ovs_key_lens[nla_type(nla)] == -1)
> -                       nlattr_set(nla, val, false);
> +               if (ovs_key_lens[nla_type(nla)] == -1)
> +                       nla_for_each_nested(nla, attr, rem)
> +                               memset(nla_data(nla), 0xff, nla_len(nla));
>                 else
> -                       memset(nla_data(nla), val, nla_len(nla));
> -       }
> -}
> +                       memset(nla_data(nla), 0xff, nla_len(nla));
>
> -static void mask_set_nlattr(struct nlattr *attr, u8 val)
> -{
> -       nlattr_set(attr, val, true);
> +               if (nla_type(nla) == OVS_KEY_ATTR_IPV6) {
> +                       struct ovs_key_ipv6 *ipv6_key = nla_data(nla);
> +
> +                       ipv6_key->ipv6_label &= htonl(0x000FFFFF);
> +               }
> +       }
>  }
>
>  /**
> @@ -926,7 +928,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
>                 if (!newmask)
>                         return -ENOMEM;
>
> -               mask_set_nlattr(newmask, 0xff);
> +               mask_set_nlattr(newmask);
>
>                 /* The userspace does not send tunnel attributes that are 0,
>                  * but we should not wildcard them nonetheless.
> --
> 1.7.10.4
>

^ permalink raw reply

* Re: [PATCH net-next v2] ipvlan: Initial check-in of the IPVLAN driver.
From: David Miller @ 2014-11-19  5:48 UTC (permalink / raw)
  To: maheshb; +Cc: netdev, edumazet, maze, chavey, thockin, brandon.philips, xemul
In-Reply-To: <CAF2d9jjrN4cDmtN_yKTe0BS+cAqE=hqgsT_6mOEBuMTTPOaPRg@mail.gmail.com>

From: Mahesh Bandewar <maheshb@google.com>
Date: Tue, 18 Nov 2014 21:27:48 -0800

> On Tue, Nov 18, 2014 at 12:54 PM, David Miller <davem@davemloft.net> wrote:
>>
>> From: Mahesh Bandewar <maheshb@google.com>
>> Date: Sun, 16 Nov 2014 22:27:14 -0800
>>
>> > +/* Define IPVL_DEBUG and set the appropriate dbg_level for debugging. */
>> > +#ifdef       IPVL_DEBUG
>> > +/*
>> > + * 1 : non-datapath debugging
>> > + * 2 : Custom
>> > + * 3 : function enters and exists.
>> > + * 4 : printk in data path (be careful!)
>> > + */
>> > +#define IPVL_DBG_LEVEL 1
>> > +#define ipvlan_dbg(level, msg...)    do { \
>> > +                                             if (level <= IPVL_DBG_LEVEL) \
>> > +                                             printk(KERN_DEBUG msg); \
>> > +                                     } while (0)
>> > +#else
>> > +#define ipvlan_dbg(level, msg...) do { ; } while (0)
>> > +#endif
>>
>> The day of having code use custom local debug logging facilities is long
>> gone, please use a standard mechanism for this rather than home cooked
>> reimplementations.
>>
> Do you mean pr_err() / pr_warn() / pr_debug() etc.? or something else?

And netdev_dbg(), etc.

And about sysfs, we're not having two ways to do the same exact
thing.  Whatever functionality you want to provide, implement it
via netlink and that's the end of it.

^ permalink raw reply

* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Eric Dumazet @ 2014-11-19  5:39 UTC (permalink / raw)
  To: Wengang Wang; +Cc: wwgwork, netdev
In-Reply-To: <1416374292-10993-1-git-send-email-wen.gang.wang@oracle.com>

On Wed, 2014-11-19 at 13:18 +0800, Wengang Wang wrote:
> When last slave of a bonding master is removed, the bonding then does not work.
> When packet_snd is called against with a master net_device, it accesses
> header_ops. In case the header_ops is not valid any longer(say ipoib module
> unloaded), it will then access an invalid memory address.
> This patch try to fix this issue by clearing header_ops when last slave
> detached.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
>  drivers/net/bonding/bond_main.c | 1 +
>  1 file changed, 1 insertion(+)

It seems you basically ignored my feedback.



Some code is doing :

[A] if (dev->header_ops) {
    ...
[B]    dev->header_ops->XXX()



Nothing prevents you doing the clear after [A] , and before [B]

^ permalink raw reply

* Re: [PATCH net-next v2] ipvlan: Initial check-in of the IPVLAN driver.
From: Mahesh Bandewar @ 2014-11-19  5:27 UTC (permalink / raw)
  To: David Miller
  Cc: linux-netdev, Eric Dumazet, Maciej Żenczykowski,
	Laurent Chavey, Tim Hockin, brandon.philips, xemul
In-Reply-To: <20141118.155437.1582768084232091503.davem@davemloft.net>

On Tue, Nov 18, 2014 at 12:54 PM, David Miller <davem@davemloft.net> wrote:
>
> From: Mahesh Bandewar <maheshb@google.com>
> Date: Sun, 16 Nov 2014 22:27:14 -0800
>
> > +/* Define IPVL_DEBUG and set the appropriate dbg_level for debugging. */
> > +#ifdef       IPVL_DEBUG
> > +/*
> > + * 1 : non-datapath debugging
> > + * 2 : Custom
> > + * 3 : function enters and exists.
> > + * 4 : printk in data path (be careful!)
> > + */
> > +#define IPVL_DBG_LEVEL 1
> > +#define ipvlan_dbg(level, msg...)    do { \
> > +                                             if (level <= IPVL_DBG_LEVEL) \
> > +                                             printk(KERN_DEBUG msg); \
> > +                                     } while (0)
> > +#else
> > +#define ipvlan_dbg(level, msg...) do { ; } while (0)
> > +#endif
>
> The day of having code use custom local debug logging facilities is long
> gone, please use a standard mechanism for this rather than home cooked
> reimplementations.
>
Do you mean pr_err() / pr_warn() / pr_debug() etc.? or something else?

> > +/* ---- Prototype declarations ---- */
> > +/* ---- ipvlan_main.c ---- */
>
> Everone knows these are prototype declarations by virtue of them being
> functions declarations in a header file.
>
> Anyone who wants to know where these are defined can run grep.
>
> So these comments are superfluous, please remove them across this
> entire submission.
>
> > +     const struct in_addr *ip4_addr = iaddr;
> > +     return jhash_1word(ip4_addr->s_addr, ipvlan_jhash_secret)
> > +            & IPVLAN_HASH_MASK;
>
> When an expression spans multiple lines, lines end rather than begin
> with an operator.
>
> > +             if (is_v6 && addr->atype == IPVL_IPV6 &&
> > +                     ipv6_addr_equal(&addr->ip6addr, iaddr))
>
> For multi-line if() statements, the second line and subsequent lines
> must begine exactly at the first column after the openning parenthesis
> of the first line.
>
> If you are purely using TAB characters to indent, you are likely doing
> it wrong.
> > +             if ((is_v6 && addr->atype == IPVL_IPV6 &&
> > +                  ipv6_addr_equal(&addr->ip6addr, iaddr))
> > +                || (!is_v6 && addr->atype == IPVL_IPV4 &&
> > +                   addr->ip4addr.s_addr == ((struct in_addr *)iaddr)->s_addr)
>
> Lines end, not begin, with operators.
>
> > --- /dev/null
> > +++ b/drivers/net/ipvlan/ipvlan_sysfs.c
>
> Please do not add sysfs configuration knobs, netlink is sufficient.

Having it does neither add complexity nor too much code, wo why not?
May be I'm missing something! But I was think that this might provide
foundation for adding other things like config locking from master's
namespace etc. (discussed in the thread) in the future.

Thanks,
--mahesh..

^ permalink raw reply

* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Wengang @ 2014-11-19  5:22 UTC (permalink / raw)
  To: Jay Vosburgh, eric.dumazet; +Cc: netdev
In-Reply-To: <1416374292-10993-1-git-send-email-wen.gang.wang@oracle.com>

Hi Jay and Eric,

Could you please review the 2nd prompt?

thanks,
wengang

于 2014年11月19日 13:18, Wengang Wang 写道:
> When last slave of a bonding master is removed, the bonding then does not work.
> When packet_snd is called against with a master net_device, it accesses
> header_ops. In case the header_ops is not valid any longer(say ipoib module
> unloaded), it will then access an invalid memory address.
> This patch try to fix this issue by clearing header_ops when last slave
> detached.
>
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
>   drivers/net/bonding/bond_main.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index c9ac06c..52a7e4b 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1730,6 +1730,7 @@ static int __bond_release_one(struct net_device *bond_dev,
>   	bond->slave_cnt--;
>   
>   	if (!bond_has_slaves(bond)) {
> +		bond->dev->header_ops = NULL;
>   		call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev);
>   		call_netdevice_notifiers(NETDEV_RELEASE, bond->dev);
>   	}

^ permalink raw reply

* [PATCH net-next v2 2/2] r8152: adjust rtl_start_rx
From: Hayes Wang @ 2014-11-19  5:20 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-95-Taiwan-albertk@realtek.com>

If there is a error for r8152_submit_rx(), add the remaining rx
buffers to the list. Then the remaining rx buffers could be
submitted later.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index df0868b..c7acadd 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2002,6 +2002,25 @@ static int rtl_start_rx(struct r8152 *tp)
 			break;
 	}
 
+	if (ret && ++i < RTL8152_MAX_RX) {
+		struct list_head rx_queue;
+		unsigned long flags;
+
+		INIT_LIST_HEAD(&rx_queue);
+
+		do {
+			struct rx_agg *agg = &tp->rx_info[i++];
+			struct urb *urb = agg->urb;
+
+			urb->actual_length = 0;
+			list_add_tail(&agg->list, &rx_queue);
+		} while (i < RTL8152_MAX_RX);
+
+		spin_lock_irqsave(&tp->rx_lock, flags);
+		list_splice_tail(&rx_queue, &tp->rx_done);
+		spin_unlock_irqrestore(&tp->rx_lock, flags);
+	}
+
 	return ret;
 }
 
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 1/2] r8152: adjust r8152_submit_rx
From: Hayes Wang @ 2014-11-19  5:20 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-95-Taiwan-albertk@realtek.com>

The behavior of handling the returned status from r8152_submit_rx()
is almost same, so let r8152_submit_rx() deal with the error
directly. This could avoid the duplicate code.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0a30fd3..df0868b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1032,7 +1032,6 @@ static void read_bulk_callback(struct urb *urb)
 	int status = urb->status;
 	struct rx_agg *agg;
 	struct r8152 *tp;
-	int result;
 
 	agg = urb->context;
 	if (!agg)
@@ -1083,16 +1082,7 @@ static void read_bulk_callback(struct urb *urb)
 		break;
 	}
 
-	result = r8152_submit_rx(tp, agg, GFP_ATOMIC);
-	if (result == -ENODEV) {
-		set_bit(RTL8152_UNPLUG, &tp->flags);
-		netif_device_detach(tp->netdev);
-	} else if (result) {
-		spin_lock(&tp->rx_lock);
-		list_add_tail(&agg->list, &tp->rx_done);
-		spin_unlock(&tp->rx_lock);
-		tasklet_schedule(&tp->tl);
-	}
+	r8152_submit_rx(tp, agg, GFP_ATOMIC);
 }
 
 static void write_bulk_callback(struct urb *urb)
@@ -1680,7 +1670,6 @@ static void rx_bottom(struct r8152 *tp)
 		int len_used = 0;
 		struct urb *urb;
 		u8 *rx_data;
-		int ret;
 
 		list_del_init(cursor);
 
@@ -1733,13 +1722,7 @@ find_next_rx:
 		}
 
 submit:
-		ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
-		if (ret && ret != -ENODEV) {
-			spin_lock_irqsave(&tp->rx_lock, flags);
-			list_add_tail(&agg->list, &tp->rx_done);
-			spin_unlock_irqrestore(&tp->rx_lock, flags);
-			tasklet_schedule(&tp->tl);
-		}
+		r8152_submit_rx(tp, agg, GFP_ATOMIC);
 	}
 }
 
@@ -1806,11 +1789,29 @@ static void bottom_half(unsigned long data)
 static
 int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags)
 {
+	int ret = 0;
+
 	usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
 			  agg->head, agg_buf_sz,
 			  (usb_complete_t)read_bulk_callback, agg);
 
-	return usb_submit_urb(agg->urb, mem_flags);
+	ret = usb_submit_urb(agg->urb, mem_flags);
+
+	if (ret == -ENODEV) {
+		set_bit(RTL8152_UNPLUG, &tp->flags);
+		netif_device_detach(tp->netdev);
+	} else if (ret) {
+		struct urb *urb = agg->urb;
+		unsigned long flags;
+
+		urb->actual_length = 0;
+		spin_lock_irqsave(&tp->rx_lock, flags);
+		list_add_tail(&agg->list, &tp->rx_done);
+		spin_unlock_irqrestore(&tp->rx_lock, flags);
+		tasklet_schedule(&tp->tl);
+	}
+
+	return ret;
 }
 
 static void rtl_drop_queued_tx(struct r8152 *tp)
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 0/2] r8152: adjust rx functions
From: Hayes Wang @ 2014-11-19  5:20 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-88-Taiwan-albertk@realtek.com>

v2:
For patch #1, set actual_length to 0 before adding the rx to the
list, when a error occurs.

For patch #2, change the flow. Stop submitting the rx if a error
occurs, and add the remaining rx to the list for submitting later.

v1:
Adjust some flows and codes which are relative to r8152_submit_rx()
and rtl_start_rx().

Hayes Wang (2):
  r8152: adjust r8152_submit_rx
  r8152: adjust rtl_start_rx

 drivers/net/usb/r8152.c | 60 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 20 deletions(-)

-- 
1.9.3

^ permalink raw reply

* [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Wengang Wang @ 2014-11-19  5:18 UTC (permalink / raw)
  To: wwgwork, netdev; +Cc: wen.gang.wang

When last slave of a bonding master is removed, the bonding then does not work.
When packet_snd is called against with a master net_device, it accesses
header_ops. In case the header_ops is not valid any longer(say ipoib module
unloaded), it will then access an invalid memory address.
This patch try to fix this issue by clearing header_ops when last slave
detached.

Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
 drivers/net/bonding/bond_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c9ac06c..52a7e4b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1730,6 +1730,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 	bond->slave_cnt--;
 
 	if (!bond_has_slaves(bond)) {
+		bond->dev->header_ops = NULL;
 		call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev);
 		call_netdevice_notifiers(NETDEV_RELEASE, bond->dev);
 	}
-- 
1.8.3.1

^ permalink raw reply related

* Re: [net-next 12/15] i40evf: make checkpatch happy
From: Joe Perches @ 2014-11-19  4:59 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Mitch Williams, netdev, nhorman, sassmann, jogreene
In-Reply-To: <1416370256-16834-13-git-send-email-jeffrey.t.kirsher@intel.com>

On Tue, 2014-11-18 at 20:10 -0800, Jeff Kirsher wrote:
> This patch is the result of running checkpatch on the i40evf driver with
> the --strict option. The vast majority of changes are adding/removing
> blank lines, aligning function parameters, and correcting over-long
> lines.

Hey Mitch, Jeff:

> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
[]
> @@ -1265,8 +1269,8 @@ int i40evf_init_interrupt_scheme(struct i40evf_adapter *adapter)
>  	}
>  
>  	dev_info(&adapter->pdev->dev, "Multiqueue %s: Queue pair count = %u",
> -		(adapter->num_active_queues > 1) ? "Enabled" :
> -		"Disabled", adapter->num_active_queues);
> +		 (adapter->num_active_queues > 1) ? "Enabled" : "Disabled",
> +		 adapter->num_active_queues);

You could add a newline to that format one day.

> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
[]
> @@ -711,7 +712,6 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
>  				"%s: Unknown event %d from pf\n",
>  				__func__, vpe->event);
>  			break;
> -
>  		}
>  		return;
>  	}
> @@ -776,7 +776,7 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
>  		break;
>  	default:
>  		dev_warn(&adapter->pdev->dev, "%s: Received unexpected message %d from PF\n",
> -			__func__, v_opcode);
> +			 __func__, v_opcode);
>  		break;
>  	} /* switch v_opcode */
>  	adapter->current_op = I40E_VIRTCHNL_OP_UNKNOWN;

And be consistent with PF vs pf usage too.

^ permalink raw reply

* [net-next 10/15] i40e: don't overload fields
From: Jeff Kirsher @ 2014-11-19  4:10 UTC (permalink / raw)
  To: davem; +Cc: Mitch Williams, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416370256-16834-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Mitch Williams <mitch.a.williams@intel.com>

Overloading the msg_size field in the arq_event_info struct is just a
bad idea. It leads to repeated bugs when the structure is used in a
loop, since the input value (buffer size) is overwritten by the output
value (actual message length).

Fix this by splitting the field into two and renaming to indicate the
actual function of each field.

Since the arq_event struct has now changed, we need to change the drivers
to support this. Note that we no longer need to initialize the buffer size
each time we go through a loop as this value is no longer destroyed by
arq processing.

In the process, we also fix a bug in i40evf_verify_api_ver where the
buffer size was not correctly reinitialized each time through the loop.

Change-ID: Ic7f9633cdd6f871f93e698dfb095e29c696f5581
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Acked-by: Shannon Nelson <shannon.nelson@intel.com>
Acked-by: Ashish Shah <ashish.n.shah@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq.c       |  6 +++---
 drivers/net/ethernet/intel/i40e/i40e_adminq.h       |  3 ++-
 drivers/net/ethernet/intel/i40e/i40e_main.c         |  7 +++----
 drivers/net/ethernet/intel/i40evf/i40e_adminq.c     |  6 +++---
 drivers/net/ethernet/intel/i40evf/i40e_adminq.h     |  3 ++-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c     |  7 +++----
 drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c | 11 +++++------
 7 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index f7f6206..5bb4914 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -980,10 +980,10 @@ i40e_status i40e_clean_arq_element(struct i40e_hw *hw,
 
 	e->desc = *desc;
 	datalen = le16_to_cpu(desc->datalen);
-	e->msg_size = min(datalen, e->msg_size);
-	if (e->msg_buf != NULL && (e->msg_size != 0))
+	e->msg_len = min(datalen, e->buf_len);
+	if (e->msg_buf != NULL && (e->msg_len != 0))
 		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
-		       e->msg_size);
+		       e->msg_len);
 
 	i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: desc and buffer:\n");
 	i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, e->msg_buf,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
index df0bd09..003a227 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
@@ -76,7 +76,8 @@ struct i40e_asq_cmd_details {
 /* ARQ event information */
 struct i40e_arq_event_info {
 	struct i40e_aq_desc desc;
-	u16 msg_size;
+	u16 msg_len;
+	u16 buf_len;
 	u8 *msg_buf;
 };
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c998d82..3ebab03 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5750,13 +5750,12 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
 	if (oldval != val)
 		wr32(&pf->hw, pf->hw.aq.asq.len, val);
 
-	event.msg_size = I40E_MAX_AQ_BUF_SIZE;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = I40E_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf)
 		return;
 
 	do {
-		event.msg_size = I40E_MAX_AQ_BUF_SIZE; /* reinit each time */
 		ret = i40e_clean_arq_element(hw, &event, &pending);
 		if (ret == I40E_ERR_ADMIN_QUEUE_NO_WORK)
 			break;
@@ -5777,7 +5776,7 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
 					le32_to_cpu(event.desc.cookie_high),
 					le32_to_cpu(event.desc.cookie_low),
 					event.msg_buf,
-					event.msg_size);
+					event.msg_len);
 			break;
 		case i40e_aqc_opc_lldp_update_mib:
 			dev_dbg(&pf->pdev->dev, "ARQ: Update LLDP MIB event received\n");
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
index 500ca21..d7e446f 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
@@ -929,10 +929,10 @@ i40e_status i40evf_clean_arq_element(struct i40e_hw *hw,
 
 	e->desc = *desc;
 	datalen = le16_to_cpu(desc->datalen);
-	e->msg_size = min(datalen, e->msg_size);
-	if (e->msg_buf != NULL && (e->msg_size != 0))
+	e->msg_len = min(datalen, e->buf_len);
+	if (e->msg_buf != NULL && (e->msg_len != 0))
 		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
-		       e->msg_size);
+		       e->msg_len);
 
 	if (i40e_is_nvm_update_op(&e->desc))
 		hw->aq.nvm_busy = false;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
index f40cfac..0d58378 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
@@ -76,7 +76,8 @@ struct i40e_asq_cmd_details {
 /* ARQ event information */
 struct i40e_arq_event_info {
 	struct i40e_aq_desc desc;
-	u16 msg_size;
+	u16 msg_len;
+	u16 buf_len;
 	u8 *msg_buf;
 };
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 4892278..d378e13 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -1632,8 +1632,8 @@ static void i40evf_adminq_task(struct work_struct *work)
 	if (adapter->flags & I40EVF_FLAG_PF_COMMS_FAILED)
 		return;
 
-	event.msg_size = I40EVF_MAX_AQ_BUF_SIZE;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = I40EVF_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf)
 		return;
 
@@ -1645,10 +1645,9 @@ static void i40evf_adminq_task(struct work_struct *work)
 
 		i40evf_virtchnl_completion(adapter, v_msg->v_opcode,
 					   v_msg->v_retval, event.msg_buf,
-					   event.msg_size);
+					   event.msg_len);
 		if (pending != 0) {
 			memset(event.msg_buf, 0, I40EVF_MAX_AQ_BUF_SIZE);
-			event.msg_size = I40EVF_MAX_AQ_BUF_SIZE;
 		}
 	} while (pending);
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 49bfdb5..422dd2d 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -92,8 +92,8 @@ int i40evf_verify_api_ver(struct i40evf_adapter *adapter)
 	enum i40e_virtchnl_ops op;
 	i40e_status err;
 
-	event.msg_size = I40EVF_MAX_AQ_BUF_SIZE;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = I40EVF_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf) {
 		err = -ENOMEM;
 		goto out;
@@ -169,15 +169,14 @@ int i40evf_get_vf_config(struct i40evf_adapter *adapter)
 
 	len =  sizeof(struct i40e_virtchnl_vf_resource) +
 		I40E_MAX_VF_VSI * sizeof(struct i40e_virtchnl_vsi_resource);
-	event.msg_size = len;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = len;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf) {
 		err = -ENOMEM;
 		goto out;
 	}
 
 	while (1) {
-		event.msg_size = len;
 		/* When the AQ is empty, i40evf_clean_arq_element will return
 		 * nonzero and this loop will terminate.
 		 */
@@ -191,7 +190,7 @@ int i40evf_get_vf_config(struct i40evf_adapter *adapter)
 	}
 
 	err = (i40e_status)le32_to_cpu(event.desc.cookie_low);
-	memcpy(adapter->vf_res, event.msg_buf, min(event.msg_size, len));
+	memcpy(adapter->vf_res, event.msg_buf, min(event.msg_len, len));
 
 	i40e_vf_parse_hw_config(hw, adapter->vf_res);
 out_alloc:
-- 
1.9.3

^ permalink raw reply related

* [net-next 15/15] i40e: trigger SW INT with no ITR wait
From: Jeff Kirsher @ 2014-11-19  4:10 UTC (permalink / raw)
  To: davem; +Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416370256-16834-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Shannon Nelson <shannon.nelson@intel.com>

Since we want the SW INT to go off as soon as possible, write the
extra bits that will turn off the ITR wait for the interrupt.

Change-ID: I6d5382ba60840fa32abb7dea17c839eb4b5f68f7
Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  5 ++++-
 drivers/net/ethernet/intel/i40e/i40e_main.c    | 10 ++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index bb1698a..b2402851 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1400,7 +1400,10 @@ static int i40e_intr_test(struct net_device *netdev, u64 *data)
 	netif_info(pf, hw, netdev, "interrupt test\n");
 	wr32(&pf->hw, I40E_PFINT_DYN_CTL0,
 	     (I40E_PFINT_DYN_CTL0_INTENA_MASK |
-	      I40E_PFINT_DYN_CTL0_SWINT_TRIG_MASK));
+	      I40E_PFINT_DYN_CTL0_SWINT_TRIG_MASK |
+	      I40E_PFINT_DYN_CTL0_ITR_INDX_MASK |
+	      I40E_PFINT_DYN_CTL0_SW_ITR_INDX_ENA_MASK |
+	      I40E_PFINT_DYN_CTL0_SW_ITR_INDX_MASK));
 	usleep_range(1000, 2000);
 	*data = (swc_old == pf->sw_int_count);
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3ebab03..3913329 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5565,11 +5565,17 @@ static void i40e_check_hang_subtask(struct i40e_pf *pf)
 			if (!(pf->flags & I40E_FLAG_MSIX_ENABLED)) {
 				wr32(&vsi->back->hw, I40E_PFINT_DYN_CTL0,
 				     (I40E_PFINT_DYN_CTL0_INTENA_MASK |
-				      I40E_PFINT_DYN_CTL0_SWINT_TRIG_MASK));
+				      I40E_PFINT_DYN_CTL0_SWINT_TRIG_MASK |
+				      I40E_PFINT_DYN_CTL0_ITR_INDX_MASK |
+				      I40E_PFINT_DYN_CTL0_SW_ITR_INDX_ENA_MASK |
+				      I40E_PFINT_DYN_CTL0_SW_ITR_INDX_MASK));
 			} else {
 				u16 vec = vsi->base_vector - 1;
 				u32 val = (I40E_PFINT_DYN_CTLN_INTENA_MASK |
-					   I40E_PFINT_DYN_CTLN_SWINT_TRIG_MASK);
+				      I40E_PFINT_DYN_CTLN_SWINT_TRIG_MASK |
+				      I40E_PFINT_DYN_CTLN_ITR_INDX_MASK |
+				      I40E_PFINT_DYN_CTLN_SW_ITR_INDX_ENA_MASK |
+				      I40E_PFINT_DYN_CTLN_SW_ITR_INDX_MASK);
 				for (i = 0; i < vsi->num_q_vectors; i++, vec++)
 					wr32(&vsi->back->hw,
 					     I40E_PFINT_DYN_CTLN(vec), val);
-- 
1.9.3

^ permalink raw reply related

* [net-next 09/15] ixgbevf: add netpoll support
From: Jeff Kirsher @ 2014-11-19  4:10 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416370256-16834-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

This patch adds ixgbevf_netpoll() a callback for .ndo_poll_controller to
allow for the VF interface to be used with netconsole.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  2 +-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 29 ++++++++++++++++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 2362001..1900ae6 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -379,7 +379,7 @@ struct ixgbevf_adapter {
 	 */
 	u32 flags;
 #define IXGBE_FLAG_IN_WATCHDOG_TASK             (u32)(1)
-#define IXGBE_FLAG_IN_NETPOLL                   (u32)(1 << 1)
+
 #define IXGBEVF_FLAG_QUEUE_RESET_REQUESTED	(u32)(1 << 2)
 
 	struct msix_entry *msix_entries;
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index dcc50cd..beb2ff1 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -344,10 +344,8 @@ static void ixgbevf_rx_skb(struct ixgbevf_q_vector *q_vector,
 		return;
 	}
 #endif /* CONFIG_NET_RX_BUSY_POLL */
-	if (!(q_vector->adapter->flags & IXGBE_FLAG_IN_NETPOLL))
-		napi_gro_receive(&q_vector->napi, skb);
-	else
-		netif_rx(skb);
+
+	napi_gro_receive(&q_vector->napi, skb);
 }
 
 /* ixgbevf_rx_checksum - indicate in skb if hw indicated a good cksum
@@ -916,12 +914,10 @@ static int ixgbevf_poll(struct napi_struct *napi, int budget)
 	else
 		per_ring_budget = budget;
 
-	adapter->flags |= IXGBE_FLAG_IN_NETPOLL;
 	ixgbevf_for_each_ring(ring, q_vector->rx)
 		clean_complete &= (ixgbevf_clean_rx_irq(q_vector, ring,
 							per_ring_budget)
 				   < per_ring_budget);
-	adapter->flags &= ~IXGBE_FLAG_IN_NETPOLL;
 
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	ixgbevf_qv_unlock_napi(q_vector);
@@ -3539,6 +3535,24 @@ static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
 	return 0;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/* Polling 'interrupt' - used by things like netconsole to send skbs
+ * without having to re-enable interrupts. It's not called while
+ * the interrupt routine is executing.
+ */
+static void ixgbevf_netpoll(struct net_device *netdev)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
+	int i;
+
+	/* if interface is down do nothing */
+	if (test_bit(__IXGBEVF_DOWN, &adapter->state))
+		return;
+	for (i = 0; i < adapter->num_rx_queues; i++)
+		ixgbevf_msix_clean_rings(0, adapter->q_vector[i]);
+}
+#endif /* CONFIG_NET_POLL_CONTROLLER */
+
 static int ixgbevf_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
@@ -3675,6 +3689,9 @@ static const struct net_device_ops ixgbevf_netdev_ops = {
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	.ndo_busy_poll		= ixgbevf_busy_poll_recv,
 #endif
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_poll_controller	= ixgbevf_netpoll,
+#endif
 };
 
 static void ixgbevf_assign_netdev_ops(struct net_device *dev)
-- 
1.9.3

^ 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