Netdev List
 help / color / mirror / Atom feed
* Re: CPSW bug with AM437x SK
From: Felipe Balbi @ 2014-10-06 14:33 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: balbi, davem, netdev, Linux OMAP Mailing List,
	Linux ARM Kernel Mailing List, Linux Kernel Mailing List
In-Reply-To: <543266F3.4090200@ti.com>

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

Hi,

On Mon, Oct 06, 2014 at 03:24:59PM +0530, Mugunthan V N wrote:
> On Friday 03 October 2014 06:34 AM, Felipe Balbi wrote:
> > [  261.177168] [<c0648d48>] (skb_panic) from [<c0565edc>] (skb_put+0x5c/0x60)
> > [  261.184415] [<c0565edc>] (skb_put) from [<c0605aac>] (unix_stream_sendmsg+0x164/0x390)
> > [  261.192712] [<c0605aac>] (unix_stream_sendmsg) from [<c055b364>] (sock_aio_write+0xdc/0xfc)
> > [  261.201475] [<c055b364>] (sock_aio_write) from [<c014c42c>] (do_sync_write+0x8c/0xb4)
> > [  261.209697] [<c014c42c>] (do_sync_write) from [<c014cf70>] (vfs_write+0x118/0x1c0)
> > [  261.217652] [<c014cf70>] (vfs_write) from [<c014d564>] (SyS_write+0x4c/0xa0)
> > [  261.225054] [<c014d564>] (SyS_write) from [<c000ed40>] (ret_fast_syscall+0x0/0x48)
> > [  261.232988] Code: e58d4008 e58de00c e59f0008 ebfff48e (e7f001f2) 
> > [  261.239378] ---[ end trace d64258d586f40104 ]---
> 
> The BT shows that the warn came from a unix socket interface, so this
> cannot be a CPSW bug, its a bug in unix socket.
> 
> Are you not seeing this issue with file system in any other media?

Have not tried with other media, but since it comes from vfs_write() and
my rootfs sits in NFS I figured "that might be the cause". Could not
reproduce this with BeagleBone Black, btw.

> I will try to reproduce this locally with my AM437x EVMsk.

alright.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: pull request: wireless-next 2014-10-03
From: John W. Linville @ 2014-10-06 15:00 UTC (permalink / raw)
  To: David Miller; +Cc: linux-wireless, netdev, linux-kernel, Larry Finger
In-Reply-To: <20141005.213853.488066614144870837.davem@davemloft.net>

On Sun, Oct 05, 2014 at 09:38:53PM -0400, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Sun, 05 Oct 2014 21:35:11 -0400 (EDT)
> 
> > From: "John W. Linville" <linville@tuxdriver.com>
> > Date: Fri, 3 Oct 2014 14:01:52 -0400
> > 
> >> Please pull tihs batch of updates intended for the 3.18 stream!
> > 
> > Pulled, thanks for the stellar pull request text, as always.
> 
> John, what's the deal with the following?  Will it be resolved by the
> driver being removed from the staging tree?
> 
> WARNING: drivers/staging/rtl8192ee/r8192ee: 'rtl_evm_dbm_jaguar' exported twice. Previous export was in drivers/net/wireless/rtlwifi/rtlwifi.ko

Yes, exactly.

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: John Fastabend @ 2014-10-06 15:01 UTC (permalink / raw)
  To: Daniel Borkmann, John Fastabend, Jesper Dangaard Brouer,
	John W. Linville, Neil Horman
  Cc: Florian Westphal, gerlitz.or, hannes, netdev, john.ronciak, amirv,
	eric.dumazet, danny.zhou
In-Reply-To: <543265A5.8000606@redhat.com>

On 10/06/2014 02:49 AM, Daniel Borkmann wrote:
> Hi John,
> 
> On 10/06/2014 03:12 AM, John Fastabend wrote:
>> On 10/05/2014 05:29 PM, Florian Westphal wrote:
>>> John Fastabend <john.fastabend@gmail.com> wrote:
>>>> There is one critical difference when running with these interfaces
>>>> vs running without them. In the normal case the af_packet module
>>>> uses a standard descriptor format exported by the af_packet user
>>>> space headers. In this model because we are working directly with
>>>> driver queues the descriptor format maps to the descriptor format
>>>> used by the device. User space applications can learn device
>>>> information from the socket option PACKET_DEV_DESC_INFO which
>>>> should provide enough details to extrapulate the descriptor formats.
>>>> Although this adds some complexity to user space it removes the
>>>> requirement to copy descriptor fields around.
>>>
>>> I find it very disappointing that we seem to have to expose such
>>> hardware specific details to userspace via hw-independent interface.
>>
>> Well it was only for convenience if it doesn't fit as a socket
>> option we can remove it. We can look up the device using the netdev
>> name from the bind call. I see your point though so if there is
>> consensus that this is not needed that is fine.
>>
>>> How big of a cost are we talking about when you say that it 'removes
>>> the requirement to copy descriptor fields'?
>>
>> This was likely a poor description. If you want to let user space
>> poll on the ring (without using system calls or interrupts) then
>> I don't see how you can _not_ expose the ring directly complete with
>> the vendor descriptor formats.
> 
> But how big is the concrete performance degradation you're seeing if you
> use an e.g. `netmap-alike` Linux-own variant as a hw-neutral interface
> that does *not* directly expose hw descriptor formats to user space?

If we don't directly expose the hardware descriptor formats then we
need to somehow kick the driver when we want it to do the copy from
the driver descriptor format to the common descriptor format.

This requires a system call as far as I can tell. Which has unwanted
overhead. I can micro-benchmark this if its helpful. But if we dredge
up Jesper's slides here we are really counting cycles so even small
numbers count if we want to hit line rate in a user space application
with 40Gpbs hardware.

> 
> With 1 core netmap does 10G line-rate on 64b; I don't know their numbers
> on 40G when run on decent hardware though.
> 
> It would really be great if we have something vendor neutral exposed as
> a stable ABI and could leverage emerging infrastructure we already have
> in the kernel such as eBPF and recent qdisc batching for raw sockets
> instead of reinventing the wheels. (Don't get me wrong, I would love to
> see AF_PACKET improved ...)

I don't think the interface is vendor specific. It does require some
knowledge of the hardware descriptor layout though. It is though vendor
neutral from my point of view. I provided the ixgbe patch simple because
I'm most familiar with it and have a NIC here. If someone wants to send me
a Mellanox NIC I can give it a try although I was hoping to recruit Or or
Amir? The only hardware feature required is flow classification to queues
which seems to be common across 10Gbps and 40/100Gbps devices. So most
of the drivers should be able to support this.

If your worried driver writers will implement the interface but not make
their descriptor formats easily available I considered putting the layout
in a header file in the uapi somewhere. Then we could just reject any
implementation that doesn't include the header file needed to use it
from user space.

With regards to leveraging eBPF and qdisc batching I don't see how this
works with direct DMA and polling. Needed to give the lowest overhead
between kernel and user space. In this case we want to use the hardware
to do the filtering that would normally be done for eBPF and for many
use cases the hardware flow classifiers is sufficient. 

We already added a qdisc bypass option I see this as taking this path
further. I believe there is room for a continuum here. For basic cases
use af_packet v1,v2 for mmap rings but using common descriptors use
af_packet v3 and set QOS_BYASS. For absolute lowest overhead and
specific applications that don't need QOS, eBPF use this interface.

Thanks.

> 
> Thanks,
> Daniel
> -- 
> 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

* [PATCH v2 07/15] virtio_net: drop config_enable
From: Michael S. Tsirkin @ 2014-10-06 15:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: kvm, Rusty Russell, virtualization, netdev
In-Reply-To: <1412608214-31944-1-git-send-email-mst@redhat.com>

Now that virtio core ensures config changes don't arrive during probing,
drop config_enable flag in virtio net.
On removal, flush is now sufficient to guarantee that no change work is
queued.

This help simplify the driver, and will allow setting DRIVER_OK earlier
without losing config change notifications.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 59caa06..743fb04 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -123,9 +123,6 @@ struct virtnet_info {
 	/* Host can handle any s/g split between our header and packet data */
 	bool any_header_sg;
 
-	/* enable config space updates */
-	bool config_enable;
-
 	/* Active statistics */
 	struct virtnet_stats __percpu *stats;
 
@@ -1408,9 +1405,6 @@ static void virtnet_config_changed_work(struct work_struct *work)
 	u16 v;
 
 	mutex_lock(&vi->config_lock);
-	if (!vi->config_enable)
-		goto done;
-
 	if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
 				 struct virtio_net_config, status, &v) < 0)
 		goto done;
@@ -1758,7 +1752,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 	}
 
 	mutex_init(&vi->config_lock);
-	vi->config_enable = true;
 	INIT_WORK(&vi->config_work, virtnet_config_changed_work);
 
 	/* If we can receive ANY GSO packets, we must allocate large ones. */
@@ -1875,17 +1868,13 @@ static void virtnet_remove(struct virtio_device *vdev)
 
 	unregister_hotcpu_notifier(&vi->nb);
 
-	/* Prevent config work handler from accessing the device. */
-	mutex_lock(&vi->config_lock);
-	vi->config_enable = false;
-	mutex_unlock(&vi->config_lock);
+	/* Make sure no work handler is accessing the device. */
+	flush_work(&vi->config_work);
 
 	unregister_netdev(vi->dev);
 
 	remove_vq_common(vi);
 
-	flush_work(&vi->config_work);
-
 	free_percpu(vi->stats);
 	free_netdev(vi->dev);
 }
@@ -1898,10 +1887,8 @@ static int virtnet_freeze(struct virtio_device *vdev)
 
 	unregister_hotcpu_notifier(&vi->nb);
 
-	/* Prevent config work handler from accessing the device */
-	mutex_lock(&vi->config_lock);
-	vi->config_enable = false;
-	mutex_unlock(&vi->config_lock);
+	/* Make sure no work handler is accessing the device */
+	flush_work(&vi->config_work);
 
 	netif_device_detach(vi->dev);
 	cancel_delayed_work_sync(&vi->refill);
@@ -1916,8 +1903,6 @@ static int virtnet_freeze(struct virtio_device *vdev)
 
 	remove_vq_common(vi);
 
-	flush_work(&vi->config_work);
-
 	return 0;
 }
 
@@ -1941,10 +1926,6 @@ static int virtnet_restore(struct virtio_device *vdev)
 
 	netif_device_attach(vi->dev);
 
-	mutex_lock(&vi->config_lock);
-	vi->config_enable = true;
-	mutex_unlock(&vi->config_lock);
-
 	rtnl_lock();
 	virtnet_set_queues(vi, vi->curr_queue_pairs);
 	rtnl_unlock();
-- 
MST

^ permalink raw reply related

* [PATCH v2 08/15] virtio-net: drop config_mutex
From: Michael S. Tsirkin @ 2014-10-06 15:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: kvm, Rusty Russell, virtualization, Cornelia Huck, netdev
In-Reply-To: <1412608214-31944-1-git-send-email-mst@redhat.com>

config_mutex served two purposes: prevent multiple concurrent config
change handlers, and synchronize access to config_enable flag.

Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1
    workqueue: make all workqueues non-reentrant
all workqueues are non-reentrant, and config_enable
is now gone.

Get rid of the unnecessary lock.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 drivers/net/virtio_net.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 743fb04..23e4a69 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -132,9 +132,6 @@ struct virtnet_info {
 	/* Work struct for config space updates */
 	struct work_struct config_work;
 
-	/* Lock for config space updates */
-	struct mutex config_lock;
-
 	/* Does the affinity hint is set for virtqueues? */
 	bool affinity_hint_set;
 
@@ -1404,7 +1401,6 @@ static void virtnet_config_changed_work(struct work_struct *work)
 		container_of(work, struct virtnet_info, config_work);
 	u16 v;
 
-	mutex_lock(&vi->config_lock);
 	if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
 				 struct virtio_net_config, status, &v) < 0)
 		goto done;
@@ -1430,7 +1426,7 @@ static void virtnet_config_changed_work(struct work_struct *work)
 		netif_tx_stop_all_queues(vi->dev);
 	}
 done:
-	mutex_unlock(&vi->config_lock);
+	return;
 }
 
 static void virtnet_config_changed(struct virtio_device *vdev)
@@ -1751,7 +1747,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 		u64_stats_init(&virtnet_stats->rx_syncp);
 	}
 
-	mutex_init(&vi->config_lock);
 	INIT_WORK(&vi->config_work, virtnet_config_changed_work);
 
 	/* If we can receive ANY GSO packets, we must allocate large ones. */
-- 
MST

^ permalink raw reply related

* [PATCH v2 09/15] virtio_net: minor cleanup
From: Michael S. Tsirkin @ 2014-10-06 15:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev, kvm, virtualization
In-Reply-To: <1412608214-31944-1-git-send-email-mst@redhat.com>

	goto done;
done:
	return;
is ugly, it was put there to make diff review easier.
replace by open-coded return.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 drivers/net/virtio_net.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 23e4a69..ef04d23 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1403,7 +1403,7 @@ static void virtnet_config_changed_work(struct work_struct *work)
 
 	if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
 				 struct virtio_net_config, status, &v) < 0)
-		goto done;
+		return;
 
 	if (v & VIRTIO_NET_S_ANNOUNCE) {
 		netdev_notify_peers(vi->dev);
@@ -1414,7 +1414,7 @@ static void virtnet_config_changed_work(struct work_struct *work)
 	v &= VIRTIO_NET_S_LINK_UP;
 
 	if (vi->status == v)
-		goto done;
+		return;
 
 	vi->status = v;
 
@@ -1425,8 +1425,6 @@ static void virtnet_config_changed_work(struct work_struct *work)
 		netif_carrier_off(vi->dev);
 		netif_tx_stop_all_queues(vi->dev);
 	}
-done:
-	return;
 }
 
 static void virtnet_config_changed(struct virtio_device *vdev)
-- 
MST

^ permalink raw reply related

* [PATCH v2 11/15] virtio_net: enable VQs early
From: Michael S. Tsirkin @ 2014-10-06 15:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: kvm, Rusty Russell, virtualization, netdev
In-Reply-To: <1412608214-31944-1-git-send-email-mst@redhat.com>

virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, virtio net violated this
rule by using receive VQs within probe.

To fix, call virtio_enable_vqs_early before using VQs.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ef04d23..430f3ae 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1792,6 +1792,8 @@ static int virtnet_probe(struct virtio_device *vdev)
 		goto free_vqs;
 	}
 
+	virtio_enable_vqs_early(vdev);
+
 	/* Last of all, set up some receive buffers. */
 	for (i = 0; i < vi->curr_queue_pairs; i++) {
 		try_fill_recv(&vi->rq[i], GFP_KERNEL);
-- 
MST

^ permalink raw reply related

* [PATCH v2 14/15] 9p/trans_virtio: enable VQs early
From: Michael S. Tsirkin @ 2014-10-06 15:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: kvm, Eric Van Hensbergen, netdev, virtualization, v9fs-developer,
	Ron Minnich, David S. Miller
In-Reply-To: <1412608214-31944-1-git-send-email-mst@redhat.com>

virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, but virtio 9p device
adds self to channel list within probe, at which point VQ can be
used in violation of the spec.

To fix, call virtio_enable_vqs_early before using VQs.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 net/9p/trans_virtio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 6940d8f..766ba48 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -575,6 +575,8 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	/* Ceiling limit to avoid denial of service attacks */
 	chan->p9_max_pages = nr_free_buffer_pages()/4;
 
+	virtio_enable_vqs_early(vdev);
+
 	mutex_lock(&virtio_9p_lock);
 	list_add_tail(&chan->chan_list, &virtio_chan_list);
 	mutex_unlock(&virtio_9p_lock);
-- 
MST

^ permalink raw reply related

* [PATCH v2 15/15] virtio_net: fix use after free on allocation failure
From: Michael S. Tsirkin @ 2014-10-06 15:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: kvm, Rusty Russell, virtualization, Cornelia Huck, netdev
In-Reply-To: <1412608214-31944-1-git-send-email-mst@redhat.com>

In the extremely unlikely event that driver initialization fails after
RX buffers are added, virtio net frees RX buffers while VQs are
still active, potentially causing device to use a freed buffer.

To fix, reset device first - same as we do on device removal.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 drivers/net/virtio_net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 430f3ae..3551417 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1830,6 +1830,8 @@ static int virtnet_probe(struct virtio_device *vdev)
 	return 0;
 
 free_recv_bufs:
+	vi->vdev->config->reset(vdev);
+
 	free_receive_bufs(vi);
 	unregister_netdev(dev);
 free_vqs:
-- 
MST

^ permalink raw reply related

* Re: [PATCH v2 07/15] virtio_net: drop config_enable
From: Cornelia Huck @ 2014-10-06 15:28 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1412608214-31944-8-git-send-email-mst@redhat.com>

On Mon, 6 Oct 2014 18:11:05 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> Now that virtio core ensures config changes don't arrive during probing,
> drop config_enable flag in virtio net.
> On removal, flush is now sufficient to guarantee that no change work is
> queued.
> 
> This help simplify the driver, and will allow setting DRIVER_OK earlier
> without losing config change notifications.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/net/virtio_net.c | 27 ++++-----------------------
>  1 file changed, 4 insertions(+), 23 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply

* Re: [Xen-devel] [PATCHv1] xen-netfront: always keep the Rx ring full of requests
From: annie li @ 2014-10-06 15:35 UTC (permalink / raw)
  To: David Vrabel; +Cc: netdev, xen-devel, Boris Ostrovsky
In-Reply-To: <1412256826-18874-1-git-send-email-david.vrabel@citrix.com>


On 2014/10/2 9:33, David Vrabel wrote:
> A full Rx ring only requires 1 MiB of memory.  This is not enough
> memory that it is useful to dynamically scale the number of Rx
> requests in the ring based on traffic rates.
>
> Keeping the ring full of Rx requests handles bursty traffic better
> than trying to converges on an optimal number of requests to keep
> filled.
>
> On a 4 core host, an iperf -P 64 -t 60 run from dom0 to a 4 VCPU guest
> improved from 5.1 Gbit/s to 5.6 Gbit/s.  Gains with more bursty
> traffic are expected to be higher.

Although removing sysfs is connected with the code change for full Rx 
ring utilization, I assume it is better to split this patch into two to 
make it simpler?

  ...snip...
>   
> +	queue->rx.req_prod_pvt = req_prod;
> +
> +	/* Not enough requests? Try again later. */
> +	if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) {
> +		mod_timer(&queue->rx_refill_timer, jiffies + (HZ/10));
> +		return;

If the previous for loop breaks because of failure of 
xennet_alloc_one_rx_buffer, then notify_remote_via_irq is missed here if 
the code returns directly.

Thanks
Annie
> +	}
> +

^ permalink raw reply

* Re: [PATCH v2 11/15] virtio_net: enable VQs early
From: Cornelia Huck @ 2014-10-06 15:39 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1412608214-31944-12-git-send-email-mst@redhat.com>

On Mon, 6 Oct 2014 18:11:19 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> virtio spec requires drivers to set DRIVER_OK before using VQs.
> This is set automatically after probe returns, virtio net violated this
> rule by using receive VQs within probe.
> 
> To fix, call virtio_enable_vqs_early before using VQs.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/net/virtio_net.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply

* [PATCH net-next] ipvs: Avoid null-pointer deref in debug code
From: Alex Gartrell @ 2014-10-06 15:46 UTC (permalink / raw)
  To: horms; +Cc: ja, dan.carpenter, lvs-devel, netdev, kernel-team, Alex Gartrell
In-Reply-To: <20141006073232.GA10073@verge.net.au>

Use daddr instead of reaching into dest.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Alex Gartrell <agartrell@fb.com>
---
 net/netfilter/ipvs/ip_vs_xmit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 91f17c1..437a366 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -316,7 +316,7 @@ __ip_vs_get_out_rt(int skb_af, struct sk_buff *skb, struct ip_vs_dest *dest,
 	if (unlikely(crosses_local_route_boundary(skb_af, skb, rt_mode,
 						  local))) {
 		IP_VS_DBG_RL("We are crossing local and non-local addresses"
-			     " daddr=%pI4\n", &dest->addr.ip);
+			     " daddr=%pI4\n", &daddr);
 		goto err_put;
 	}
 
@@ -458,7 +458,7 @@ __ip_vs_get_out_rt_v6(int skb_af, struct sk_buff *skb, struct ip_vs_dest *dest,
 	if (unlikely(crosses_local_route_boundary(skb_af, skb, rt_mode,
 						  local))) {
 		IP_VS_DBG_RL("We are crossing local and non-local addresses"
-			     " daddr=%pI6\n", &dest->addr.in6);
+			     " daddr=%pI6\n", daddr);
 		goto err_put;
 	}
 
-- 
Alex Gartrell <agartrell@fb.com>


^ permalink raw reply related

* Re: [PATCH net-next] ipvs: Avoid null-pointer deref in debug code
From: Alex Gartrell @ 2014-10-06 15:56 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: horms, dan.carpenter, lvs-devel, netdev, kernel-team
In-Reply-To: <alpine.LFD.2.11.1410060940470.1738@ja.home.ssi.bg>

Hey Julian,

On 10/05/2014 11:49 PM, Julian Anastasov wrote:
>
> 	You have to print the "daddr" variable as
> it was done before your patchset in the
> "Stopping traffic to %s address, dest: %p..." message
> because dest is not present in all cases, for example,
> for *bypass_xmit. Other places provide cp->daddr but
> for backup server some conns can live without cp->dest.

I've sent an updated patch that does this but I have some questions 
about other stuff that I find mildly confusing.  Specifically I didn't 
realize until looking at the call sites that !dest || daddr = 
dest->addr.ip (but maybe I'm wrong?)

If that's the case, why do we have the following line in __ip_vs_get_out_rt?

                 daddr = dest->addr.ip;

If that's /always/ true then we should add the following line or a 
comment to the same effect to clarify

	BUG_ON(dest && dest->addr.ip != daddr);

If that's not intended to always be true, then should the patch be the 
following?

	...%pI4", dest ? &dest->addr.ip : &daddr);

Thanks,
-- 
Alex Gartrell <agartrell@fb.com>

^ permalink raw reply

* Re: [Xen-devel] [PATCHv1] xen-netfront: always keep the Rx ring full of requests
From: David Vrabel @ 2014-10-06 16:00 UTC (permalink / raw)
  To: annie li; +Cc: netdev, xen-devel, Boris Ostrovsky
In-Reply-To: <5432B6D2.9030503@oracle.com>

On 06/10/14 16:35, annie li wrote:
> 
> On 2014/10/2 9:33, David Vrabel wrote:
>> A full Rx ring only requires 1 MiB of memory.  This is not enough
>> memory that it is useful to dynamically scale the number of Rx
>> requests in the ring based on traffic rates.
>>
>> Keeping the ring full of Rx requests handles bursty traffic better
>> than trying to converges on an optimal number of requests to keep
>> filled.
>>
>> On a 4 core host, an iperf -P 64 -t 60 run from dom0 to a 4 VCPU guest
>> improved from 5.1 Gbit/s to 5.6 Gbit/s.  Gains with more bursty
>> traffic are expected to be higher.
> 
> Although removing sysfs is connected with the code change for full Rx
> ring utilization, I assume it is better to split this patch into two to
> make it simpler?

I don't see how splitting the patch would be an improvement.

>>   +    queue->rx.req_prod_pvt = req_prod;
>> +
>> +    /* Not enough requests? Try again later. */
>> +    if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) {
>> +        mod_timer(&queue->rx_refill_timer, jiffies + (HZ/10));
>> +        return;
> 
> If the previous for loop breaks because of failure of
> xennet_alloc_one_rx_buffer, then notify_remote_via_irq is missed here if
> the code returns directly.

This is deliberate -- there's no point notifying the backend if there
aren't enough requests for the next packet.  Since we don't know what
the next packet might be we assume it's the largest possible.

David

^ permalink raw reply

* Re: [PATCH net-next 0/2] sunvnet: Packet processing in non-interrupt context.
From: Sowmini Varadhan @ 2014-10-06 16:04 UTC (permalink / raw)
  To: David Miller; +Cc: raghuram.kothakota, netdev
In-Reply-To: <20141003.120802.1213573830649867131.davem@davemloft.net>

> I think you should be able to get rid of all of the in-driver
> locking in the fast paths.
> 
> NAPI ->poll() is non-reentrant, so all RX processing occurs
> strictly in a serialized environment.
> 
> Once you do TX reclaim in NAPI context, then all you have to do is
> take the generic netdev TX queue lock during the evaluation of whether
> to wakeup the TX queue or not.  Worst case you need to hold the
> TX netdev queue lock across the whole TX reclaim operation.
> 
> The VIO lock really ought to be entirely superfluous in the data
> paths.

A few clarifications, since there are more driver-examples using NAPI for
Rx than for Tx reclaim

so I can move the LDC_EVENT_RESET/LDC_EVENT_UP processing code into the 
napi callback, and that enables the removal of irqsave/restore for
vio.lock from vio_port_up at the least (I do this conditional on
in_softirq() so as to not perturb vdc code at the moment)

But there are still a lot of irqsaves at the ldc layer for the lp lock.
I dont know if these can/should be optimized out. 

I looked at tg3 for a template on how to use NAPI in the TX path
The analog of the tg3_poll_work->tg3_tx invocation is probably the
maybe_tx_wakeup call triggered from the Rx side vnet processing,
which, with NAPI happens naturally from softirq context (no need for
extra tasklet). 

Regarding rcu locking of port_list and the hash in struct vnet_port,
the thorn here is that vnet_set_rx_mode may end up allocating a
vnet_mcast_entry as part of __update_mc_list
(there may be a different bug here in that it assumes that the 
first entry is the switch_port, and this is the only switch_port)
I dont know of a simple way to avoid that (a rwlock just for this
function?!).

But we still need to hold the vio lock around the ldc_write 
(and also around dring write) in vnet_start_xmit, right?

--Sowmini

^ permalink raw reply

* [PATCH net-next] net/mlx4_en: remove NETDEV_TX_BUSY
From: Eric Dumazet @ 2014-10-06 16:30 UTC (permalink / raw)
  To: Amir Vadai
  Cc: David S. Miller, Eric Dumazet, netdev, Yevgeny Petrilin,
	Or Gerlitz, Ido Shamay
In-Reply-To: <1412599904.11091.66.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <edumazet@google.com>

Drivers should avoid NETDEV_TX_BUSY as much as possible.

They should stop the tx queue before qdisc even tries to push another
packet, to avoid requeues.

For a driver supporting skb->xmit_more, this is likely to be a prereq
anyway, otherwise we could have a tx deadlock : We need to force a
doorbell if TX ring is full.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |   48 +++++++++----------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 92a7cf46d9af9260d601a0d65ec5bba4..44004e331e4e760f18cfa69ef99a2b2b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -706,6 +706,7 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 	void *fragptr = NULL;
 	bool bounce = false;
 	bool send_doorbell;
+	bool stop_queue;
 	bool inline_ok;
 	u32 ring_cons;
 
@@ -735,30 +736,6 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (vlan_tx_tag_present(skb))
 		vlan_tag = vlan_tx_tag_get(skb);
 
-	/* Check available TXBBs And 2K spare for prefetch */
-	if (unlikely(((int)(ring->prod - ring_cons)) >
-		     ring->size - HEADROOM - MAX_DESC_TXBBS)) {
-		/* every full Tx ring stops queue */
-		netif_tx_stop_queue(ring->tx_queue);
-		ring->queue_stopped++;
-
-		/* If queue was emptied after the if, and before the
-		 * stop_queue - need to wake the queue, or else it will remain
-		 * stopped forever.
-		 * Need a memory barrier to make sure ring->cons was not
-		 * updated before queue was stopped.
-		 */
-		wmb();
-
-		ring_cons = ACCESS_ONCE(ring->cons);
-		if (unlikely(((int)(ring->prod - ring_cons)) <=
-			     ring->size - HEADROOM - MAX_DESC_TXBBS)) {
-			netif_tx_wake_queue(ring->tx_queue);
-			ring->wake_queue++;
-		} else {
-			return NETDEV_TX_BUSY;
-		}
-	}
 
 	prefetchw(&ring->tx_queue->dql);
 
@@ -929,6 +906,13 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	skb_tx_timestamp(skb);
 
+	/* Check available TXBBs And 2K spare for prefetch */
+	stop_queue = (int)(ring->prod - ring_cons) >
+		      ring->size - HEADROOM - MAX_DESC_TXBBS;
+	if (unlikely(stop_queue)) {
+		netif_tx_stop_queue(ring->tx_queue);
+		ring->queue_stopped++;
+	}
 	send_doorbell = !skb->xmit_more || netif_xmit_stopped(ring->tx_queue);
 
 	real_size = (real_size / 16) & 0x3f;
@@ -973,6 +957,22 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 
+	if (unlikely(stop_queue)) {
+		/* If queue was emptied after the if (stop_queue) , and before
+		 * the netif_tx_stop_queue() - need to wake the queue,
+		 * or else it will remain stopped forever.
+		 * Need a memory barrier to make sure ring->cons was not
+		 * updated before queue was stopped.
+		 */
+		smp_rmb();
+
+		ring_cons = ACCESS_ONCE(ring->cons);
+		if (unlikely(((int)(ring->prod - ring_cons)) <=
+			     ring->size - HEADROOM - MAX_DESC_TXBBS)) {
+			netif_tx_wake_queue(ring->tx_queue);
+			ring->wake_queue++;
+		}
+	}
 	return NETDEV_TX_OK;
 
 tx_drop_unmap:

^ permalink raw reply related

* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Jesper Dangaard Brouer @ 2014-10-06 16:35 UTC (permalink / raw)
  To: John Fastabend
  Cc: brouer, Daniel Borkmann, John Fastabend, John W. Linville,
	Neil Horman, Florian Westphal, gerlitz.or, hannes, netdev,
	john.ronciak, amirv, eric.dumazet, danny.zhou
In-Reply-To: <5432AEE0.9000600@intel.com>


On Mon, 06 Oct 2014 08:01:52 -0700 John Fastabend <john.r.fastabend@intel.com> wrote:
 
> This requires a system call as far as I can tell. Which has unwanted
> overhead. I can micro-benchmark this if its helpful. But if we dredge
> up Jesper's slides here we are really counting cycles so even small
> numbers count if we want to hit line rate in a user space application
> with 40Gpbs hardware.

The micro-benchmarked syscall[2] cost is approx 42 ns [1] (when
disabling CONFIG_AUDITSYSCALL else its approx 88ns), which is
significant compared to the 10G wirespeed smallest packet size budget
of 67.2ns.

See:
 [1] http://netoptimizer.blogspot.dk/2014/05/the-calculations-10gbits-wirespeed.html
 [2] https://github.com/netoptimizer/network-testing/blob/master/src/syscall_overhead.c

[...] 
> We already added a qdisc bypass option I see this as taking this path
> further. I believe there is room for a continuum here. For basic cases
> use af_packet v1,v2 for mmap rings but using common descriptors use
> af_packet v3 and set QOS_BYASS. For absolute lowest overhead and
> specific applications that don't need QOS, eBPF use this interface.

Well, after the qdisc bulking changes, when bulking kicks in then the
qdisc path is faster than the qdisc bypass (measured with trafgen).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH] net: bcmgenet: fix increase rx_read_ptr
From: Florian Fainelli @ 2014-10-06 16:45 UTC (permalink / raw)
  To: Jaedon Shin; +Cc: netdev
In-Reply-To: <1412564726-40192-1-git-send-email-jaedon.shin@gmail.com>

On 10/05/2014 08:05 PM, Jaedon Shin wrote:
> The rx_read_ptr must increase after using it.

Your commit message is too terse, you need to explain why you think the
current code is bad, and how your patch is fixing it.

One possible thing that I see is that we might be off by one in how we
use the enet_cb versus how we read the HW packet descriptor.

> 
> Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 5cc9cae..b47db5e 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1282,9 +1282,6 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_priv *priv,
>  
>  		rxpktprocessed++;
>  
> -		priv->rx_read_ptr++;
> -		priv->rx_read_ptr &= (priv->num_rx_bds - 1);
> -
>  		/* We do not have a backing SKB, so we do not have a
>  		 * corresponding DMA mapping for this incoming packet since
>  		 * bcmgenet_rx_refill always either has both skb and mapping or
> @@ -1399,6 +1396,9 @@ refill:
>  		err = bcmgenet_rx_refill(priv, cb);
>  		if (err)
>  			netif_err(priv, rx_err, dev, "Rx refill failed\n");
> +
> +		priv->rx_read_ptr++;
> +		priv->rx_read_ptr &= (priv->num_rx_bds - 1);
>  	}
>  
>  	return rxpktprocessed;
> 

^ permalink raw reply

* Re: randconfig build error with next-20141001, in drivers/i2c/algos/i2c-algo-bit.c
From: Randy Dunlap @ 2014-10-06 16:52 UTC (permalink / raw)
  To: Oliver Hartkopp, Jim Davis, Stephen Rothwell
  Cc: linux-next, Stephane Grosjean, linux-i2c, netdev@vger.kernel.org,
	linux-can
In-Reply-To: <54324D68.6070309@hartkopp.net>

On 10/06/14 01:06, Oliver Hartkopp wrote:
> Hello all,
> 
> just to get it right:
> 
> So far it looks like this in linux/drivers/net/can/sja1000/Kconfig
> 
> config CAN_PEAK_PCIEC
>         bool "PEAK PCAN-ExpressCard Cards"
>         depends on CAN_PEAK_PCI
>         select I2C
>         select I2C_ALGOBIT
> 
> If one would change the
> 
>         select I2C
> 
> into
> 
>         depends on I2C
> 
> IMHO the CAN_PEAK_PCIEC hardware would *only* be visible and selectable when
> I2C was selected before (from anyone else?).

That is correct.

> So what it wrong on the current Kconfig entry?
> Is 'select' deprecated?

No, it's not deprecated.  It's just dangerous.  and driver configs should not
enable entire subsystems via 'select'.

> Or did randconfig generate a configuration that would not be possible by
> properly generating the config file with 'make menuconfig' ??

randconfig generated a config for another driver which causes a build error,
not for a CAN driver.  The CAN driver does not have a build error AFAIK.
Its Kconfig is just doing something with a very big & ugly stick.


> Please explain.
> 
> Thanks,
> Oliver
> 
> On 10/02/2014 01:26 AM, Randy Dunlap wrote:
>> On 10/01/14 14:37, Jim Davis wrote:
>>> Building with the attached random configuration file,
>>
>> Also:
>> warning: (CAN_PEAK_PCIEC && SFC && IGB && VIDEO_TW68 && DRM && FB_DDC && FB_VIA) selects I2C_ALGOBIT which has unmet direct dependencies (I2C)
>>
>>> drivers/i2c/algos/i2c-algo-bit.c: In function ‘i2c_bit_add_bus’:
>>> drivers/i2c/algos/i2c-algo-bit.c:658:33: error: ‘i2c_add_adapter’
>>> undeclared (first use in this function)
>>>   return __i2c_bit_add_bus(adap, i2c_add_adapter);
>>>                                  ^
>>> drivers/i2c/algos/i2c-algo-bit.c:658:33: note: each undeclared
>>> identifier is reported only once for each function it appears in
>>> drivers/i2c/algos/i2c-algo-bit.c: In function ‘i2c_bit_add_numbered_bus’:
>>> drivers/i2c/algos/i2c-algo-bit.c:664:33: error:
>>> ‘i2c_add_numbered_adapter’ undeclared (first use in this function)
>>>   return __i2c_bit_add_bus(adap, i2c_add_numbered_adapter);
>>>                                  ^
>>>   CC      net/openvswitch/actions.o
>>> drivers/i2c/algos/i2c-algo-bit.c: In function ‘i2c_bit_add_bus’:
>>> drivers/i2c/algos/i2c-algo-bit.c:659:1: warning: control reaches end of non-void
>>>  function [-Wreturn-type]
>>>  }
>>>  ^
>>> drivers/i2c/algos/i2c-algo-bit.c: In function ‘i2c_bit_add_numbered_bus’:
>>> drivers/i2c/algos/i2c-algo-bit.c:665:1: warning: control reaches end of non-void
>>>  function [-Wreturn-type]
>>>  }
>>>  ^
>>> make[3]: *** [drivers/i2c/algos/i2c-algo-bit.o] Error 1
>>
>> In drivers/media/pci/tw68/Kconfig, VIDEO_TW68 should depend on I2C in order
>> to make it safe to select I2C_ALGOBIT.
>>
>> In drivers/net/can/sja1000/Kconfig, CAN_PEAK_PCIEC should depend on I2C
>> instead of selecting I2C (and change the help text).


-- 
~Randy

^ permalink raw reply

* Re: [net-next PATCH v1 1/3] net: sched: remove tcf_proto from ematch calls
From: Cong Wang @ 2014-10-06 16:52 UTC (permalink / raw)
  To: John Fastabend
  Cc: Cong Wang, David Miller, netdev, Jamal Hadi Salim, Eric Dumazet
In-Reply-To: <20141006042751.6010.86647.stgit@nitbit.x32>

On Sun, Oct 5, 2014 at 9:27 PM, John Fastabend <john.fastabend@gmail.com> wrote:
> This removes the tcf_proto argument from the ematch code paths that
> only need it to reference the net namespace. This allows simplifying
> qdisc code paths especially when we need to tear down the ematch
> from an RCU callback. In this case we can not guarentee that the
> tcf_proto structure is still valid.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

The code looks cleaner now. :)

Acked-by: Cong Wang <cwang@twopensource.com>

^ permalink raw reply

* Re: [Patch net-next] net_sched: refactor out tcf_exts
From: Cong Wang @ 2014-10-06 16:56 UTC (permalink / raw)
  To: John Fastabend
  Cc: Cong Wang, netdev, Jamal Hadi Salim, John Fastabend,
	David S. Miller
In-Reply-To: <543206B4.7090504@gmail.com>

On Sun, Oct 5, 2014 at 8:04 PM, John Fastabend <john.fastabend@gmail.com> wrote:
> On 10/05/2014 06:47 PM, John Fastabend wrote:
>>
>> On 10/03/2014 03:51 PM, Cong Wang wrote:
>>>
>>> As Jamal pointed it out, tcf_exts is really unnecessary,
>>> we can definitely refactor it out without losing any functionality.
>>> This could also remove an indirect layer which makes the code
>>> much easier to read.
>>>
>>> This patch:
>>>
>>> 1) moves exts->action and exts->police into tp->ops, since they
>>> are statically assigned
>>>
>>> 2) moves exts->actions list head out
>>>
>>> 3) removes exts->type, act->type does the same thing
>>>
>>> 4) renames tcf_exts_*() functions to tcf_act_*()
>>>
>>> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
>>> Cc: John Fastabend <john.r.fastabend@intel.com>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>> ---
>>
>>
>> Looks OK to me and removes a layer of abstraction without changing
>> the code much. This is going to conflict with my series so I'll hold
>> off resubmitting it until this is dealt with. I need to respin that
>> ematch fix up to drop the ingress lock.
>>
>> Acked-by: John Fastabend <john.r.fastabend@intel.com>
>>
>> [...]
>>
>
> But after running my test kit I see a null pointer dereference
> in cls_cgroup in tcf_act_change().
>
> Looks like you dropped an initializer...

Oops, yeah, should have an INIT_LIST_HEAD()...

I will send an update.

Thanks!

^ permalink raw reply

* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Stephen Hemminger @ 2014-10-06 16:55 UTC (permalink / raw)
  To: John Fastabend
  Cc: dborkman, fw, gerlitz.or, hannes, netdev, john.ronciak, amirv,
	eric.dumazet, danny.zhou
In-Reply-To: <20141006000629.32055.2295.stgit@nitbit.x32>

On Sun, 05 Oct 2014 17:06:31 -0700
John Fastabend <john.fastabend@gmail.com> wrote:

> This patch adds a net_device ops to split off a set of driver queues
> from the driver and map the queues into user space via mmap. This
> allows the queues to be directly manipulated from user space. For
> raw packet interface this removes any overhead from the kernel network
> stack.
> 
> Typically in an af_packet interface a packet_type handler is
> registered and used to filter traffic to the socket and do other
> things such as fan out traffic to multiple sockets. In this case the
> networking stack is being bypassed so this code is not run. So the
> hardware must push the correct traffic to the queues obtained from
> the ndo callback ndo_split_queue_pairs().
> 
> Fortunately there is already a flow classification interface which
> is part of the ethtool command set, ETHTOOL_SRXCLSRLINS. It is
> currently supported by multiple drivers including sfc, mlx4, niu,
> ixgbe, and i40e. Supporting some way to steer traffic to a queue
> is the _only_ hardware requirement to support the interface, plus
> the driver needs to implement the correct ndo ops. A follow on
> patch adds support for ixgbe but we expect at least the subset of
> drivers implementing ETHTOOL_SRXCLSRLINS to be implemented later.
> 
> The interface is driven over an af_packet socket which we believe
> is the most natural interface to use. Because it is already used
> for raw packet interfaces which is what we are providing here.
>  The high level flow for this interface looks like:
> 
> 	bind(fd, &sockaddr, sizeof(sockaddr));
> 
> 	/* Get the device type and info */
> 	getsockopt(fd, SOL_PACKET, PACKET_DEV_DESC_INFO, &def_info,
> 		   &optlen);
> 
> 	/* With device info we can look up descriptor format */
> 
> 	/* Get the layout of ring space offset, page_sz, cnt */
> 	getsockopt(fd, SOL_PACKET, PACKET_DEV_QPAIR_MAP_REGION_INFO,
> 		   &info, &optlen);
> 
> 	/* request some queues from the driver */
> 	setsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
> 		   &qpairs_info, sizeof(qpairs_info));
> 
> 	/* if we let the driver pick us queues learn which queues
>          * we were given
>          */
> 	getsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
> 		   &qpairs_info, sizeof(qpairs_info));
> 
> 	/* And mmap queue pairs to user space */
> 	mmap(NULL, info.tp_dev_bar_sz, PROT_READ | PROT_WRITE,
> 	     MAP_SHARED, fd, 0);
> 
> 	/* Now we have some user space queues to read/write to*/
> 
> There is one critical difference when running with these interfaces
> vs running without them. In the normal case the af_packet module
> uses a standard descriptor format exported by the af_packet user
> space headers. In this model because we are working directly with
> driver queues the descriptor format maps to the descriptor format
> used by the device. User space applications can learn device
> information from the socket option PACKET_DEV_DESC_INFO which
> should provide enough details to extrapulate the descriptor formats.
> Although this adds some complexity to user space it removes the
> requirement to copy descriptor fields around.
> 
> The formats are usually provided by the device vendor documentation
> If folks want I can provide a follow up patch to provide the formats
> in a .h file in ./include/uapi/linux/ for ease of use. I have access
> to formats for ixgbe and mlx drivers other driver owners would need to
> provide their formats.
> 
> We tested this interface using traffic generators and doing basic
> L2 forwarding tests on ixgbe devices. Our tests use a set of patches
> to DPDK to enable an interface using this socket interfaace. With
> this interface we can xmit/receive @ line rate from a test user space
> application on a single core.
> 
> Additionally we have a set of DPDK patches to enable DPDK with this
> interface. DPDK can be downloaded @ dpdk.org although as I hope is
> clear from above DPDK is just our paticular test environment we
> expect other libraries could be built on this interface.
> 
> Signed-off-by: Danny Zhou <danny.zhou@intel.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

I like the ability to share a device between kernel and user mode networking.
The model used for DPDK for this is really ugly and fragile/broken.
Your proposal assumes that you fully trust the user mode networking application
which is not a generally safe assumption.

A device can DMA from/to any arbitrary physical memory. 
And it would be hard to use IOMMU to protect because the
IOMMU doesn't know that the difference between the applications queue and
the rest of the queues.

At least with DPDK you can use VFIO, and you are claiming the whole device to
allow protection against random memory being read/written.

^ permalink raw reply

* Re: [Patch net-next] net_sched: refactor out tcf_exts
From: Cong Wang @ 2014-10-06 16:56 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: Cong Wang, netdev, John Fastabend, David S. Miller
In-Reply-To: <54327B09.9070204@mojatatu.com>

On Mon, Oct 6, 2014 at 4:20 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> Sorry for being annoying.
> Cong - here's a simple test that will test for both .police and .action
> for one classifier. If you have time please run it when updating your
> patch.
>

Sure. Thanks for sharing it!

^ permalink raw reply

* Re: [net-next PATCH v1 2/3] net: sched: cls_cgroup tear down exts and ematch from rcu callback
From: Cong Wang @ 2014-10-06 17:01 UTC (permalink / raw)
  To: John Fastabend
  Cc: Cong Wang, David Miller, netdev, Jamal Hadi Salim, Eric Dumazet
In-Reply-To: <20141006042819.6010.32857.stgit@nitbit.x32>

On Sun, Oct 5, 2014 at 9:28 PM, John Fastabend <john.fastabend@gmail.com> wrote:
> It is not RCU safe to destroy the action chain while there
> is a possibility of readers accessing it. Move this code
> into the rcu callback using the same rcu callback used in the
> code patch to make a change to head.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

Acked-by: Cong Wang <cwang@twopensource.com>

^ permalink raw reply


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