* Re: [PATCH 1/2] net: Allow ethtool to set interface in loopback mode.
From: Michał Mirosław @ 2011-05-04 11:15 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Matt Carlson, David Miller, netdev, Michael Chan, Ben Hutchings,
Tom Herbert
In-Reply-To: <1304471935-402-2-git-send-email-maheshb@google.com>
On Tue, May 03, 2011 at 06:18:54PM -0700, Mahesh Bandewar wrote:
> This patch enables ethtool to set the loopback mode on a given interface.
> By configuring the interface in loopback mode in conjunction with a policy
> route / rule, a userland application can stress the egress / ingress path
> exposing the flows of the change in progress and potentially help developer(s)
> understand the impact of those changes without even sending a packet out
> on the network.
>
> Following set of commands illustrates one such example -
> a) ip -4 addr add 192.168.1.1/24 dev eth1
> b) ip -4 rule add from all iif eth1 lookup 250
> c) ip -4 route add local 0/0 dev lo proto kernel scope host table 250
> d) arp -Ds 192.168.1.100 eth1
> e) arp -Ds 192.168.1.200 eth1
> f) sysctl -w net.ipv4.ip_nonlocal_bind=1
> g) sysctl -w net.ipv4.conf.all.accept_local=1
> # Assuming that the machine has 8 cores
> h) taskset 000f netserver -L 192.168.1.200
> i) taskset 00f0 netperf -t TCP_CRR -L 192.168.1.100 -H 192.168.1.200 -l 30
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
> include/linux/netdevice.h | 3 ++-
> net/core/ethtool.c | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d5de66a..e7244ed 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1067,6 +1067,7 @@ struct net_device {
> #define NETIF_F_RXHASH (1 << 28) /* Receive hashing offload */
> #define NETIF_F_RXCSUM (1 << 29) /* Receive checksumming offload */
> #define NETIF_F_NOCACHE_COPY (1 << 30) /* Use no-cache copyfromuser */
> +#define NETIF_F_LOOPBACK (1 << 31) /* Enable loopback */
[...]
Just for correctness: you could add this flag to loopback's dev->features.
It's just an aesthetics point, though.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCHv3 2/2] tg3: Allow ethtool to enable/disable loopback.
From: Michał Mirosław @ 2011-05-04 11:11 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Matt Carlson, David Miller, netdev, Michael Chan, Ben Hutchings,
Tom Herbert
In-Reply-To: <1304471935-402-3-git-send-email-maheshb@google.com>
On Tue, May 03, 2011 at 06:18:55PM -0700, Mahesh Bandewar wrote:
> This patch adds tg3_set_features() to handle loopback mode. Currently the
> capability is added for the devices which support internal MAC loopback mode.
> So when enabled, it enables internal-MAC loopback.
[...]
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 7c7c9a8..46de633 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -6319,6 +6319,51 @@ static u32 tg3_fix_features(struct net_device *dev, u32 features)
> return features;
> }
>
> +static int tg3_set_features(struct net_device *dev, u32 features)
> +{
> + struct tg3 *tp = netdev_priv(dev);
> + u32 cur_mode = 0;
> + int err = 0;
> +
> + if (!netif_running(dev)) {
> + err = -EAGAIN;
> + goto sfeatures_out;
> + }
netdev_update_features() is not designed to handle -EAGAIN from
ndo_set_features callback. It might be useful to implement this
handling, but in this case you should just return 0 and check
dev->features in ndo_open callback.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH V4 4/8]vhost: vhost TX zero-copy support
From: Michael S. Tsirkin @ 2011-05-04 9:56 UTC (permalink / raw)
To: Shirley Ma
Cc: David Miller, Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev,
kvm, linux-kernel
In-Reply-To: <1304496684.20660.84.camel@localhost.localdomain>
On Wed, May 04, 2011 at 01:11:24AM -0700, Shirley Ma wrote:
> This patch maintains the outstanding userspace buffers in the
> sequence it is delivered to vhost. The outstanding userspace buffers
> will be marked as done once the lower device buffers DMA has finished.
> This is monitored through last reference of kfree_skb callback. Two
> buffer index are used for this purpose.
>
> The vhost passes the userspace buffers info to lower device skb
> through message control. Since there will be some done DMAs when
> entering vhost handle_tx. The worse case is all buffers in the vq are
> in pending/done status, so we need to notify guest to release DMA done
> buffers first before get any new buffers from the vq.
>
> Signed-off-by: Shirley <xma@us.ibm.com>
Looks good overall. Some nits to iron out below.
> ---
>
> drivers/vhost/net.c | 30 +++++++++++++++++++++++++++-
> drivers/vhost/vhost.c | 50
> ++++++++++++++++++++++++++++++++++++++++++++++++-
> drivers/vhost/vhost.h | 10 +++++++++
> 3 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 2f7c76a..c403afb 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -32,6 +32,8 @@
> * Using this limit prevents one virtqueue from starving others. */
> #define VHOST_NET_WEIGHT 0x80000
>
> +#define MAX_ZEROCOPY_PEND 64
> +
Pls document what the above is. Also scope with VHOST_
> enum {
> VHOST_NET_VQ_RX = 0,
> VHOST_NET_VQ_TX = 1,
> @@ -129,6 +131,7 @@ static void handle_tx(struct vhost_net *net)
> int err, wmem;
> size_t hdr_size;
> struct socket *sock;
> + struct skb_ubuf_info pend;
>
> /* TODO: check that we are running from vhost_worker? */
> sock = rcu_dereference_check(vq->private_data, 1);
> @@ -151,6 +154,10 @@ static void handle_tx(struct vhost_net *net)
> hdr_size = vq->vhost_hlen;
>
> for (;;) {
> + /* Release DMAs done buffers first */
> + if (sock_flag(sock->sk, SOCK_ZEROCOPY))
> + vhost_zerocopy_signal_used(vq);
> +
> head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
> ARRAY_SIZE(vq->iov),
> &out, &in,
> @@ -166,6 +173,12 @@ static void handle_tx(struct vhost_net *net)
> set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> break;
> }
> + /* If more outstanding DMAs, queue the work */
> + if (sock_flag(sock->sk, SOCK_ZEROCOPY) &&
> + (atomic_read(&vq->refcnt) > MAX_ZEROCOPY_PEND)) {
> + vhost_poll_queue(&vq->poll);
Well, this just keeps polling, doesn't it?
If you want to wait until # of DMAs is below MAX_ZEROCOPY_PEND,
you'll need to do the queueing from some callback.
Something like this: when refcnt is above 2 * MAX_ZEROCOPY_PEND,
stop submitting and wait until some are freed.
BTW, can the socket poll wakeup do the job?
> + break;
> + }
> if (unlikely(vhost_enable_notify(vq))) {
> vhost_disable_notify(vq);
> continue;
> @@ -188,17 +201,30 @@ static void handle_tx(struct vhost_net *net)
> iov_length(vq->hdr, s), hdr_size);
> break;
> }
> + /* use msg_control to pass vhost zerocopy ubuf info to skb */
> + if (sock_flag(sock->sk, SOCK_ZEROCOPY)) {
> + pend.callback = vhost_zerocopy_callback;
> + pend.arg = vq;
> + pend.desc = vq->upend_idx;
> + msg.msg_control = &pend;
> + msg.msg_controllen = sizeof(pend);
> + vq->heads[vq->upend_idx].id = head;
> + vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
> + atomic_inc(&vq->refcnt);
> + }
> /* TODO: Check specific error and bomb out unless ENOBUFS? */
> err = sock->ops->sendmsg(NULL, sock, &msg, len);
> if (unlikely(err < 0)) {
> - vhost_discard_vq_desc(vq, 1);
> + if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
> + vhost_discard_vq_desc(vq, 1);
> tx_poll_start(net, sock);
> break;
> }
> if (err != len)
> pr_debug("Truncated TX packet: "
> " len %d != %zd\n", err, len);
> - vhost_add_used_and_signal(&net->dev, vq, head, 0);
> + if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
> + vhost_add_used_and_signal(&net->dev, vq, head, 0);
> total_len += len;
> if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
> vhost_poll_queue(&vq->poll);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 2ab2912..3048953 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -174,6 +174,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->call_ctx = NULL;
> vq->call = NULL;
> vq->log_ctx = NULL;
> + vq->upend_idx = 0;
> + vq->done_idx = 0;
> + atomic_set(&vq->refcnt, 0);
> }
>
> static int vhost_worker(void *data)
> @@ -230,7 +233,7 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev
> *dev)
> UIO_MAXIOV, GFP_KERNEL);
> dev->vqs[i].log = kmalloc(sizeof *dev->vqs[i].log * UIO_MAXIOV,
> GFP_KERNEL);
> - dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads *
> + dev->vqs[i].heads = kzalloc(sizeof *dev->vqs[i].heads *
> UIO_MAXIOV, GFP_KERNEL);
Do we really need to zero it all out? We generally tried to only
init what is necessary ...
> if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
> @@ -385,10 +388,41 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
> return 0;
> }
>
Pls document what the below does.
> +void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
> +{
> + int i, j = 0;
> +
> + i = vq->done_idx;
> + while (i != vq->upend_idx) {
> + /* len = 1 means DMA done */
Hmm. Guests aren't likely to use len 1 in practice,
but I think it's better to support this.
I'd suggest sticking extra stuff in id, IIRC only values
< vq size are legal there, anything else we can use.
Also, pls add some defines for special values, better than
a comment:
if (len == VHOST_DMA_DONE_LEN)
> + if (vq->heads[i].len == 1) {
> + /* reset len = 0 */
> + vq->heads[i].len = 0;
> + i = (i + 1) % UIO_MAXIOV;
> + ++j;
> + } else
> + break;
> + }
> + if (j) {
Pls add some comments to explain the logic here.
> + if (i > vq->done_idx)
> + vhost_add_used_n(vq, &vq->heads[vq->done_idx], j);
> + else {
> + vhost_add_used_n(vq, &vq->heads[vq->done_idx],
> + UIO_MAXIOV - vq->done_idx);
> + vhost_add_used_n(vq, vq->heads, i);
> + }
> + vq->done_idx = i;
> + vhost_signal(vq->dev, vq);
> + atomic_sub(j, &vq->refcnt);
> + }
> +}
> +
> /* Caller should have device mutex */
> void vhost_dev_cleanup(struct vhost_dev *dev)
> {
> int i;
> + unsigned long begin = jiffies;
> +
>
> for (i = 0; i < dev->nvqs; ++i) {
> if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
> @@ -405,6 +439,11 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> eventfd_ctx_put(dev->vqs[i].call_ctx);
> if (dev->vqs[i].call)
> fput(dev->vqs[i].call);
> + /* wait for all lower device DMAs done, then notify guest */
> + while (atomic_read(&dev->vqs[i].refcnt)) {
> + if (time_after(jiffies, begin + 5 * HZ))
Hmm, does this actually busy-wait? Let's at least sleep here.
Or maybe some wakeup scheme can be cooked up.
For example, have a kref with release function that signals some
completion.
> + vhost_zerocopy_signal_used(&dev->vqs[i]);
> + }
> vhost_vq_reset(dev, dev->vqs + i);
> }
> vhost_dev_free_iovecs(dev);
> @@ -1416,3 +1455,12 @@ void vhost_disable_notify(struct vhost_virtqueue
> *vq)
> vq_err(vq, "Failed to enable notification at %p: %d\n",
> &vq->used->flags, r);
> }
> +
> +void vhost_zerocopy_callback(struct sk_buff *skb)
> +{
> + int idx = skb_shinfo(skb)->ubuf.desc;
> + struct vhost_virtqueue *vq = skb_shinfo(skb)->ubuf.arg;
> +
> + /* set len = 1 to mark this desc buffers done DMA */
> + vq->heads[idx].len = 1;
> +}
So any kind of callback like that, that goes into the skb,
will be racy wrt module unloading because module can go away
after you mark dma done and before this function returns.
Solution is to have a core function that does the
final signalling (e.g. sock_wfree is in core).
Would be nice to fix, even though this race is
completely theoretical, I don't believe it will
trigger in practice.
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index b3363ae..ec032a0 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -108,6 +108,14 @@ struct vhost_virtqueue {
> /* Log write descriptors */
> void __user *log_base;
> struct vhost_log *log;
> + /* vhost zerocopy support */
> + atomic_t refcnt; /* num of outstanding zerocopy DMAs */
> + /* index of zerocopy pending DMA buffers */
> + int upend_idx;
> + /* index of zerocopy done DMA buffers, but not notify guest yet */
> + int done_idx;
Pls try to find more descriptive names for the above,
and clarify the comments: I could not tell what do the
comments mean.
upend_idx seems to be a copy of avail idx?
done_idx is ... ?
> + /* notify vhost zerocopy DMA buffers has done in lower device */
Do you mean 'notify vhost that zerocopy DMA is complete'?
> + void (*callback)(struct sk_buff *);
Is this actually used?
If yes rename it zerocopy_dma_done or something like this?
> };
>
> struct vhost_dev {
> @@ -154,6 +162,8 @@ bool vhost_enable_notify(struct vhost_virtqueue *);
>
> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> unsigned int log_num, u64 len);
> +void vhost_zerocopy_callback(struct sk_buff *skb);
> +void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
>
> #define vq_err(vq, fmt, ...) do { \
> pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
>
^ permalink raw reply
* Re: [RFC v3 02/10] Revert "lsm: Remove the socket_post_accept() hook"
From: Samir Bellabes @ 2011-05-04 8:50 UTC (permalink / raw)
To: Tetsuo Handa
Cc: paul.moore, linux-security-module, linux-kernel, netdev,
netfilter-devel, hadi, kaber, zbr, root
In-Reply-To: <201105041128.BAB13061.LMHVtOSOQOFFJF@I-love.SAKURA.ne.jp>
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
> Paul Moore wrote:
>> On Tuesday, May 03, 2011 10:24:15 AM Samir Bellabes wrote:
>> > snet needs to reintroduce this hook, as it was designed to be: a hook for
>> > updating security informations on objects.
>>
>> Looking at this and 5/10 again, it seems that you should be able to do what
>> you need with the sock_graft() hook. Am I missing something?
>>
>> My apologies if we've already discussed this approach previously ...
>
> Third problem (though independent with security_sock_graft()) is that
> snet_do_send_event() ignores snet_nl_send_event() failure.
using snet_do_send_event() means that system is sending data to
userspace. the system is not waiting for a verdict from userspace.
If error occurs, we actually loose the information data.
I may be able to write a solution which try to send the data again, but
we need a exit solution for this loop (a number of try ?).
^ permalink raw reply
* [PATCH V4 5/8]macvtap: macvtap TX zero-copy support
From: Shirley Ma @ 2011-05-04 8:14 UTC (permalink / raw)
To: David Miller, mst, Eric Dumazet, Avi Kivity, Arnd Bergmann
Cc: netdev, kvm, linux-kernel
Only when buffer size is greater than GOODCOPY_LEN (256), macvtap
enables zero-copy.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
---
drivers/net/macvtap.c | 126 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 115 insertions(+), 11 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 6696e56..e8bc5ff 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -60,6 +60,7 @@ static struct proto macvtap_proto = {
*/
static dev_t macvtap_major;
#define MACVTAP_NUM_DEVS 65536
+#define GOODCOPY_LEN 256
static struct class *macvtap_class;
static struct cdev macvtap_cdev;
@@ -340,6 +341,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
{
struct net *net = current->nsproxy->net_ns;
struct net_device *dev = dev_get_by_index(net, iminor(inode));
+ struct macvlan_dev *vlan = netdev_priv(dev);
struct macvtap_queue *q;
int err;
@@ -369,6 +371,16 @@ static int macvtap_open(struct inode *inode, struct file *file)
q->flags = IFF_VNET_HDR | IFF_NO_PI | IFF_TAP;
q->vnet_hdr_sz = sizeof(struct virtio_net_hdr);
+ /*
+ * so far only VM uses macvtap, enable zero copy between guest
+ * kernel and host kernel when lower device supports high memory
+ * DMA
+ */
+ if (vlan) {
+ if (vlan->lowerdev->features & NETIF_F_ZEROCOPY)
+ sock_set_flag(&q->sk, SOCK_ZEROCOPY);
+ }
+
err = macvtap_set_queue(dev, file, q);
if (err)
sock_put(&q->sk);
@@ -433,6 +445,80 @@ static inline struct sk_buff *macvtap_alloc_skb(struct sock *sk, size_t prepad,
return skb;
}
+/* set skb frags from iovec, this can move to core network code for reuse */
+static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ int offset, size_t count)
+{
+ int len = iov_length(from, count) - offset;
+ int copy = skb_headlen(skb);
+ int size, offset1 = 0;
+ int i = 0;
+ skb_frag_t *f;
+
+ /* Skip over from offset */
+ while (offset >= from->iov_len) {
+ offset -= from->iov_len;
+ ++from;
+ --count;
+ }
+
+ /* copy up to skb headlen */
+ while (copy > 0) {
+ size = min_t(unsigned int, copy, from->iov_len - offset);
+ if (copy_from_user(skb->data + offset1, from->iov_base + offset,
+ size))
+ return -EFAULT;
+ if (copy > size) {
+ ++from;
+ --count;
+ }
+ copy -= size;
+ offset1 += size;
+ offset = 0;
+ }
+
+ if (len == offset1)
+ return 0;
+
+ while (count--) {
+ struct page *page[MAX_SKB_FRAGS];
+ int num_pages;
+ unsigned long base;
+
+ len = from->iov_len - offset1;
+ if (!len) {
+ offset1 = 0;
+ ++from;
+ continue;
+ }
+ base = (unsigned long)from->iov_base + offset1;
+ size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ num_pages = get_user_pages_fast(base, size, 0, &page[i]);
+ if ((num_pages != size) ||
+ (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags))
+ /* put_page is in skb free */
+ return -EFAULT;
+ skb->data_len += len;
+ skb->len += len;
+ skb->truesize += len;
+ while (len) {
+ f = &skb_shinfo(skb)->frags[i];
+ f->page = page[i];
+ f->page_offset = base & ~PAGE_MASK;
+ f->size = min_t(int, len, PAGE_SIZE - f->page_offset);
+ skb_shinfo(skb)->nr_frags++;
+ /* increase sk_wmem_alloc */
+ atomic_add(f->size, &skb->sk->sk_wmem_alloc);
+ base += f->size;
+ len -= f->size;
+ i++;
+ }
+ offset1 = 0;
+ ++from;
+ }
+ return 0;
+}
+
/*
* macvtap_skb_from_vnet_hdr and macvtap_skb_to_vnet_hdr should
* be shared with the tun/tap driver.
@@ -515,17 +601,19 @@ static int macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
/* Get packet from user space buffer */
-static ssize_t macvtap_get_user(struct macvtap_queue *q,
- const struct iovec *iv, size_t count,
- int noblock)
+static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
+ const struct iovec *iv, unsigned long total_len,
+ size_t count, int noblock)
{
struct sk_buff *skb;
struct macvlan_dev *vlan;
- size_t len = count;
+ unsigned long len = total_len;
int err;
struct virtio_net_hdr vnet_hdr = { 0 };
int vnet_hdr_len = 0;
+ int copylen, zerocopy;
+ zerocopy = sock_flag(&q->sk, SOCK_ZEROCOPY) && (len > GOODCOPY_LEN);
if (q->flags & IFF_VNET_HDR) {
vnet_hdr_len = q->vnet_hdr_sz;
@@ -552,12 +640,28 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
if (unlikely(len < ETH_HLEN))
goto err;
- skb = macvtap_alloc_skb(&q->sk, NET_IP_ALIGN, len, vnet_hdr.hdr_len,
- noblock, &err);
+ if (zerocopy)
+ /* There are 256 bytes to be copied in skb, so there is enough
+ * room for skb expand head in case it is used.
+ * The rest buffer is mapped from userspace.
+ */
+ copylen = GOODCOPY_LEN;
+ else
+ copylen = len;
+
+ skb = macvtap_alloc_skb(&q->sk, NET_IP_ALIGN, copylen,
+ vnet_hdr.hdr_len, noblock, &err);
if (!skb)
goto err;
- err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len, len);
+ if (zerocopy)
+ err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
+ else
+ err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
+ len);
+ if (sock_flag(&q->sk, SOCK_ZEROCOPY))
+ memcpy(&skb_shinfo(skb)->ubuf, m->msg_control,
+ sizeof(struct skb_ubuf_info));
if (err)
goto err_kfree;
@@ -579,7 +683,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
kfree_skb(skb);
rcu_read_unlock_bh();
- return count;
+ return total_len;
err_kfree:
kfree_skb(skb);
@@ -601,8 +705,8 @@ static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
ssize_t result = -ENOLINK;
struct macvtap_queue *q = file->private_data;
- result = macvtap_get_user(q, iv, iov_length(iv, count),
- file->f_flags & O_NONBLOCK);
+ result = macvtap_get_user(q, NULL, iv, iov_length(iv, count), count,
+ file->f_flags & O_NONBLOCK);
return result;
}
@@ -815,7 +919,7 @@ static int macvtap_sendmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t total_len)
{
struct macvtap_queue *q = container_of(sock, struct macvtap_queue, sock);
- return macvtap_get_user(q, m->msg_iov, total_len,
+ return macvtap_get_user(q, m, m->msg_iov, total_len, m->msg_iovlen,
m->msg_flags & MSG_DONTWAIT);
}
^ permalink raw reply related
* [PATCH V4 4/8]vhost: vhost TX zero-copy support
From: Shirley Ma @ 2011-05-04 8:11 UTC (permalink / raw)
To: David Miller, mst, Eric Dumazet, Avi Kivity, Arnd Bergmann
Cc: netdev, kvm, linux-kernel
This patch maintains the outstanding userspace buffers in the
sequence it is delivered to vhost. The outstanding userspace buffers
will be marked as done once the lower device buffers DMA has finished.
This is monitored through last reference of kfree_skb callback. Two
buffer index are used for this purpose.
The vhost passes the userspace buffers info to lower device skb
through message control. Since there will be some done DMAs when
entering vhost handle_tx. The worse case is all buffers in the vq are
in pending/done status, so we need to notify guest to release DMA done
buffers first before get any new buffers from the vq.
Signed-off-by: Shirley <xma@us.ibm.com>
---
drivers/vhost/net.c | 30 +++++++++++++++++++++++++++-
drivers/vhost/vhost.c | 50
++++++++++++++++++++++++++++++++++++++++++++++++-
drivers/vhost/vhost.h | 10 +++++++++
3 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 2f7c76a..c403afb 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -32,6 +32,8 @@
* Using this limit prevents one virtqueue from starving others. */
#define VHOST_NET_WEIGHT 0x80000
+#define MAX_ZEROCOPY_PEND 64
+
enum {
VHOST_NET_VQ_RX = 0,
VHOST_NET_VQ_TX = 1,
@@ -129,6 +131,7 @@ static void handle_tx(struct vhost_net *net)
int err, wmem;
size_t hdr_size;
struct socket *sock;
+ struct skb_ubuf_info pend;
/* TODO: check that we are running from vhost_worker? */
sock = rcu_dereference_check(vq->private_data, 1);
@@ -151,6 +154,10 @@ static void handle_tx(struct vhost_net *net)
hdr_size = vq->vhost_hlen;
for (;;) {
+ /* Release DMAs done buffers first */
+ if (sock_flag(sock->sk, SOCK_ZEROCOPY))
+ vhost_zerocopy_signal_used(vq);
+
head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
ARRAY_SIZE(vq->iov),
&out, &in,
@@ -166,6 +173,12 @@ static void handle_tx(struct vhost_net *net)
set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
break;
}
+ /* If more outstanding DMAs, queue the work */
+ if (sock_flag(sock->sk, SOCK_ZEROCOPY) &&
+ (atomic_read(&vq->refcnt) > MAX_ZEROCOPY_PEND)) {
+ vhost_poll_queue(&vq->poll);
+ break;
+ }
if (unlikely(vhost_enable_notify(vq))) {
vhost_disable_notify(vq);
continue;
@@ -188,17 +201,30 @@ static void handle_tx(struct vhost_net *net)
iov_length(vq->hdr, s), hdr_size);
break;
}
+ /* use msg_control to pass vhost zerocopy ubuf info to skb */
+ if (sock_flag(sock->sk, SOCK_ZEROCOPY)) {
+ pend.callback = vhost_zerocopy_callback;
+ pend.arg = vq;
+ pend.desc = vq->upend_idx;
+ msg.msg_control = &pend;
+ msg.msg_controllen = sizeof(pend);
+ vq->heads[vq->upend_idx].id = head;
+ vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
+ atomic_inc(&vq->refcnt);
+ }
/* TODO: Check specific error and bomb out unless ENOBUFS? */
err = sock->ops->sendmsg(NULL, sock, &msg, len);
if (unlikely(err < 0)) {
- vhost_discard_vq_desc(vq, 1);
+ if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
+ vhost_discard_vq_desc(vq, 1);
tx_poll_start(net, sock);
break;
}
if (err != len)
pr_debug("Truncated TX packet: "
" len %d != %zd\n", err, len);
- vhost_add_used_and_signal(&net->dev, vq, head, 0);
+ if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
+ vhost_add_used_and_signal(&net->dev, vq, head, 0);
total_len += len;
if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
vhost_poll_queue(&vq->poll);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ab2912..3048953 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -174,6 +174,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call_ctx = NULL;
vq->call = NULL;
vq->log_ctx = NULL;
+ vq->upend_idx = 0;
+ vq->done_idx = 0;
+ atomic_set(&vq->refcnt, 0);
}
static int vhost_worker(void *data)
@@ -230,7 +233,7 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev
*dev)
UIO_MAXIOV, GFP_KERNEL);
dev->vqs[i].log = kmalloc(sizeof *dev->vqs[i].log * UIO_MAXIOV,
GFP_KERNEL);
- dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads *
+ dev->vqs[i].heads = kzalloc(sizeof *dev->vqs[i].heads *
UIO_MAXIOV, GFP_KERNEL);
if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
@@ -385,10 +388,41 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
return 0;
}
+void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
+{
+ int i, j = 0;
+
+ i = vq->done_idx;
+ while (i != vq->upend_idx) {
+ /* len = 1 means DMA done */
+ if (vq->heads[i].len == 1) {
+ /* reset len = 0 */
+ vq->heads[i].len = 0;
+ i = (i + 1) % UIO_MAXIOV;
+ ++j;
+ } else
+ break;
+ }
+ if (j) {
+ if (i > vq->done_idx)
+ vhost_add_used_n(vq, &vq->heads[vq->done_idx], j);
+ else {
+ vhost_add_used_n(vq, &vq->heads[vq->done_idx],
+ UIO_MAXIOV - vq->done_idx);
+ vhost_add_used_n(vq, vq->heads, i);
+ }
+ vq->done_idx = i;
+ vhost_signal(vq->dev, vq);
+ atomic_sub(j, &vq->refcnt);
+ }
+}
+
/* Caller should have device mutex */
void vhost_dev_cleanup(struct vhost_dev *dev)
{
int i;
+ unsigned long begin = jiffies;
+
for (i = 0; i < dev->nvqs; ++i) {
if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
@@ -405,6 +439,11 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
eventfd_ctx_put(dev->vqs[i].call_ctx);
if (dev->vqs[i].call)
fput(dev->vqs[i].call);
+ /* wait for all lower device DMAs done, then notify guest */
+ while (atomic_read(&dev->vqs[i].refcnt)) {
+ if (time_after(jiffies, begin + 5 * HZ))
+ vhost_zerocopy_signal_used(&dev->vqs[i]);
+ }
vhost_vq_reset(dev, dev->vqs + i);
}
vhost_dev_free_iovecs(dev);
@@ -1416,3 +1455,12 @@ void vhost_disable_notify(struct vhost_virtqueue
*vq)
vq_err(vq, "Failed to enable notification at %p: %d\n",
&vq->used->flags, r);
}
+
+void vhost_zerocopy_callback(struct sk_buff *skb)
+{
+ int idx = skb_shinfo(skb)->ubuf.desc;
+ struct vhost_virtqueue *vq = skb_shinfo(skb)->ubuf.arg;
+
+ /* set len = 1 to mark this desc buffers done DMA */
+ vq->heads[idx].len = 1;
+}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index b3363ae..ec032a0 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -108,6 +108,14 @@ struct vhost_virtqueue {
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
+ /* vhost zerocopy support */
+ atomic_t refcnt; /* num of outstanding zerocopy DMAs */
+ /* index of zerocopy pending DMA buffers */
+ int upend_idx;
+ /* index of zerocopy done DMA buffers, but not notify guest yet */
+ int done_idx;
+ /* notify vhost zerocopy DMA buffers has done in lower device */
+ void (*callback)(struct sk_buff *);
};
struct vhost_dev {
@@ -154,6 +162,8 @@ bool vhost_enable_notify(struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
+void vhost_zerocopy_callback(struct sk_buff *skb);
+void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply related
* [PATCH V4 3/8] skbuff: Add userspace buffers support in skb (zero-copy)
From: Shirley Ma @ 2011-05-04 8:06 UTC (permalink / raw)
To: David Miller, mst, Eric Dumazet, Avi Kivity, Arnd Bergmann
Cc: netdev, kvm, linux-kernel
This patch adds userspace buffers support in skb shared info. A new
struct skb_ubuf_info is needed to maintain the userspace buffers argument
and index, a callback is used to notify userspace to release the
buffers once lower device has done DMA (Last reference to that skb
has gone). This kind of skb has a 256 bytes copied data in head to
make sure we have enough room for head expanding; and mapped the rest of
userspace buffers in skb frags.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
---
include/linux/skbuff.h | 26 ++++++++++++++++++++++++++
net/core/skbuff.c | 13 +++++++++++++
2 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d0ae90a..025de5c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -189,6 +189,18 @@ enum {
SKBTX_DRV_NEEDS_SK_REF = 1 << 3,
};
+/*
+ * The callback notifies userspace to release buffers when skb DMA is done in
+ * lower device, the skb last reference should be 0 when calling this.
+ * The desc is used to track userspace buffer index.
+ */
+struct skb_ubuf_info {
+ /* support buffers allocation from userspace */
+ void (*callback)(struct sk_buff *);
+ void *arg;
+ size_t desc;
+};
+
/* This data is invariant across clones and lives at
* the end of the header data, ie. at skb->end.
*/
@@ -211,6 +223,10 @@ struct skb_shared_info {
/* Intermediate layers must ensure that destructor_arg
* remains valid until skb destructor */
void * destructor_arg;
+
+ /* DMA mapping from/to userspace buffers */
+ struct skb_ubuf_info ubuf;
+
/* must be last field, see pskb_expand_head() */
skb_frag_t frags[MAX_SKB_FRAGS];
};
@@ -2261,5 +2277,15 @@ static inline void skb_checksum_none_assert(struct sk_buff *skb)
}
bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off);
+
+/*
+ * skb_ubuf - is the buffer from userspace
+ * @skb: buffer to check
+ */
+static inline int skb_ubuf(const struct sk_buff *skb)
+{
+ return (skb_shinfo(skb)->ubuf.callback != NULL);
+}
+
#endif /* __KERNEL__ */
#endif /* _LINUX_SKBUFF_H */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7ebeed0..9cbd3fc 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -210,6 +210,8 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
shinfo = skb_shinfo(skb);
memset(shinfo, 0, offsetof(struct skb_shared_info, dataref));
atomic_set(&shinfo->dataref, 1);
+ shinfo->ubuf.callback = NULL;
+ shinfo->ubuf.arg = NULL;
kmemcheck_annotate_variable(shinfo->destructor_arg);
if (fclone) {
@@ -328,6 +330,14 @@ static void skb_release_data(struct sk_buff *skb)
put_page(skb_shinfo(skb)->frags[i].page);
}
+ /*
+ * if skb buf is from userspace, we need to notify the caller
+ * the lower device DMA has done;
+ */
+ if (skb_shinfo(skb)->ubuf.callback) {
+ skb_shinfo(skb)->ubuf.callback(skb);
+ skb_shinfo(skb)->ubuf.callback = NULL;
+ }
if (skb_has_frag_list(skb))
skb_drop_fraglist(skb);
@@ -480,6 +490,9 @@ bool skb_recycle_check(struct sk_buff *skb, int skb_size)
if (irqs_disabled())
return false;
+ if (skb_ubuf(skb))
+ return false;
+
if (skb_is_nonlinear(skb) || skb->fclone != SKB_FCLONE_UNAVAILABLE)
return false;
^ permalink raw reply related
* [PATCH V4 2/8] netdevice.h: Add a new zerocopy device flag
From: Shirley Ma @ 2011-05-04 7:55 UTC (permalink / raw)
To: David Miller, mst, Eric Dumazet, Avi Kivity, Arnd Bergmann
Cc: netdev, kvm, linux-kernel
Signed-off-by: Shirley Ma <xma@us.ibm.com>
---
include/linux/netdevice.h | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0249fe7..0808f1e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1067,6 +1067,16 @@ struct net_device {
#define NETIF_F_RXHASH (1 << 28) /* Receive hashing offload */
#define NETIF_F_RXCSUM (1 << 29) /* Receive checksumming offload */
+/*
+ * Bit 31 is for device to map userspace buffers -- zerocopy
+ * Device can set this flag when it supports HIGHDMA.
+ * Device can't recycle this kind of skb buffers.
+ * There are 256 bytes copied, the rest of buffers are mapped.
+ * The userspace callback should only be called when last reference to this skb
+ * is gone.
+ */
+#define NETIF_F_ZEROCOPY (1 << 31)
+
/* Segmentation offload features */
#define NETIF_F_GSO_SHIFT 16
#define NETIF_F_GSO_MASK 0x00ff0000
^ permalink raw reply related
* [PATCH V4 1/8] sock.h: Add a new sock zero-copy flag
From: Shirley Ma @ 2011-05-04 7:53 UTC (permalink / raw)
To: David Miller, mst, Eric Dumazet, Avi Kivity, Arnd Bergmann
Cc: netdev, kvm, linux-kernel
Signed-off-by: Shirley Ma <xma@us.ibm.com>
---
include/net/sock.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 01810a3..ab09097 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -562,6 +562,7 @@ enum sock_flags {
SOCK_TIMESTAMPING_SYS_HARDWARE, /* %SOF_TIMESTAMPING_SYS_HARDWARE */
SOCK_FASYNC, /* fasync() active */
SOCK_RXQ_OVFL,
+ SOCK_ZEROCOPY, /* buffers from userspace */
};
static inline void sock_copy_flags(struct sock *nsk, struct sock *osk)
^ permalink raw reply related
* [PATCH V4 0/8] macvtap/vhost TX zero-copy support
From: Shirley Ma @ 2011-05-04 7:48 UTC (permalink / raw)
To: David Miller, mst, Eric Dumazet, Avi Kivity, Arnd Bergmann
Cc: netdev, kvm, linux-kernel
This patchset add supports for TX zero-copy between guest and host
kernel through vhost. It significantly reduces CPU utilization on the
local host on which the guest is located (It reduced 30-50% CPU usage
for vhost thread for single stream test). The patchset is based on
previous submission and comments from the community regarding when/how
to handle guest kernel buffers to be released. This is the simplest
approach I can think of after comparing with several other solutions.
This patchset has integrated V3 review comments from the community:
1. Add more comments on how to use device ZEROCOPY flag;
2. Change device ZEROCOPY to available bit 31
3. Fix skb header linear allocation when virtio_net GSO is not enabled
This patchset includes:
1/8: Add a new sock zero-copy flag, SOCK_ZEROCOPY;
2/8: Add a new device flag, NETIF_F_ZEROCOPY for lower level device
support zero-copy;
3/8: Add a new struct skb_ubuf_info in skb_share_info for userspace
buffers release callback when lower device DMA has done for that skb,
which is the last reference count gone;
4/8: Add vhost zero-copy callback in vhost when skb last refcnt is gone;
add vhost_zerocopy_signal_used to notify guest to release TX skb
buffers.
5/8: Add macvtap zero-copy in lower device when sending packet is
greater than 256 bytes to make sure there is enough room for expanding
skb head.
6/8: Add Chelsio 10Gb NIC to zero-copy feature flag
7/8: Add Intel 10Gb NIC zero-copy feature flag
8/8: Add Emulex 10Gb NIC zero-copy feature flag
The patchset is built against most recent linux 2.6.39-rc5. It has
passed netperf/netserver multiple streams stress test on above NICs.
Single TCP_STREAM 120 secs test results over ixgbe 10Gb NIC results:
Message BW(Gb/s)qemu-kvm (NumCPU)vhost-net(NumCPU) PerfTop irq/s
4K 7408.57 92.1% 22.6% 1229
4K(Orig)4913.17 118.1% 84.1% 2086
8K 9129.90 89.3% 23.3% 1141
8K(Orig)7094.55 115.9% 84.7% 2157
16K 9178.81 89.1% 23.3% 1139
16K(Orig)8927.1 118.7% 83.4% 2262
64K 9171.43 88.4% 24.9% 1253
64K(Orig)9085.85 115.9% 82.4% 2229
For message size less or equal than 2K, there is a known KVM guest TX
overrun issue. With this zero-copy patch, the issue becomes more severe,
guest io_exits has tripled than before, so the performance is not good.
Once the TX overrun problem has been addressed, I will retest the small
message size performance.
Thanks
Shirley
^ permalink raw reply
* Re: [PATCH] usbnet: Transfer of maintainership
From: Oliver Neukum @ 2011-05-04 7:36 UTC (permalink / raw)
To: Richard Cochran; +Cc: davem, netdev, USB list
In-Reply-To: <20110504054511.GA3362@riccoc20.at.omicron.at>
Am Mittwoch, 4. Mai 2011, 07:45:11 schrieb Richard Cochran:
> On Fri, Apr 29, 2011 at 02:19:04PM +0200, Oliver Neukum wrote:
>
> > USB "USBNET" DRIVER FRAMEWORK
> > -M: David Brownell <dbrownell@users.sourceforge.net>
> > +M: Oliver Neukum <oneukum@suse.de>
>
> Oliver,
>
> We have been looking at usbnet and a have question.
>
> Usbnet doesn't use either phylib or napi, but I think the reason is
> probably purely historical. Is there a technical reason why phylib or
> napi won't work with usbnet devices?
Hi,
phylib is for historical reasons, although many devices won't
give you much access to the phy.
NAPI however is technically not very meaningful for USB
devices as you cannot poll them. So the central concept
behind NAPI doesn't apply.
Regards
Oliver
--
- - -
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
- - -
^ permalink raw reply
* Re: [PATCH] usbnet: runtime pm: fix out of memory
From: Oliver Neukum @ 2011-05-04 7:26 UTC (permalink / raw)
To: Ming Lei
Cc: David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <BANLkTinZK_qjO7u+ckG_83_paVyMeyrgPw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Am Mittwoch, 4. Mai 2011, 08:59:41 schrieb Ming Lei:
> Hi,
>
> 2011/5/3 Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>:
>
> > Do the devices in question use cdc_ether?
>
> No, the device is smsc95xx, which is compound device and is
> integrated into pandaboard.
OK,
in this case:
Acked-by: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
Regards
Oliver
--
- - -
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
- - -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Subhasish Ghosh @ 2011-05-04 7:13 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Netdev-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0, open list,
CAN NETWORK DRIVERS, Marc Kleine-Budde, m-watkins-l0cyMroinI0,
Wolfgang Grandegger
In-Reply-To: <201104271525.28512.arnd-r2nGTMty4D4@public.gmane.org>
> On Wednesday 27 April 2011, Subhasish Ghosh wrote:
>> >
>> > - Use just one value per sysfs file
>>
>> SG - I felt adding entry for each mbx_id will clutter the sysfs.
>> Is it ok to do that.
>
> That is probably not much better either.
>
> Note also that every sysfs file needs to come with associated
> documentation in Documentation/ABI/*/ to make sure that users
> will know exactly how the file is meant to work.
>
> Why do you need to export these values in the first place? Is
> it just for debugging or do you expect all CAN user space
> to look at this?
>
> If it's for debugging, please don't export the files through sysfs.
> Depending on how useful the data is to regular users, you can
> still export it through a debugfs file in that case, which has
> much less strict rules.
>
> If the file is instead meant as part of the regular operation of
> the device, it should not be in debugfs but probably be integrated
> into the CAN socket interface, so that users don't need to work
> with two different ways of getting to the device (socket and sysfs).
>
CAN requires mail box IDs to be programmed in. But, the socket
CAN subsystem supports only software filtering of the mail box IDs.
So, the mail box IDs programmed into socket CAN during initialization
does not propagate into the hardware. This is planned to be a future
implementation in Socket CAN.
In our case, we support hardware filtering, to work around with this,
Wolfgang (Socket CAN owner) suggested that we implement
this using sysfs.
These setting are not for debugging, but to program the mail box IDs
into the hardware.
^ permalink raw reply
* Re: [PATCH] usbnet: runtime pm: fix out of memory
From: Ming Lei @ 2011-05-04 6:59 UTC (permalink / raw)
To: Oliver Neukum; +Cc: David Miller, netdev, linux-usb
In-Reply-To: <201105030953.44131.oneukum@suse.de>
Hi,
2011/5/3 Oliver Neukum <oneukum@suse.de>:
> Do the devices in question use cdc_ether?
No, the device is smsc95xx, which is compound device and is
integrated into pandaboard.
> The problem I see with this patch is that cdc_ether uses .reset_resume = usbnet_resume
> Therefore the device will not have been reset from the viewpoint of the device, yet
> the device may be open, so the bug would strike again.
>
> It seems to me that this patch is not wrong as such, but incomplete.
Since it is not cdc device, so the patch should be complete, the idea behind
the patch is reasonable: only start to schedule urb for data packets after the
interface is opened.
Even from the comment below in usbnet_open, it is a generic fix for
this kind of issue, we should always put the device into a 'known safe" state
before starting communication.
int usbnet_open (struct net_device *net)
......
// put into "known safe" state
......
thanks,
--
Ming Lei
^ permalink raw reply
* [PATCH] net: add mac_pton() for parsing MAC address
From: Alexey Dobriyan @ 2011-05-04 6:15 UTC (permalink / raw)
To: davem; +Cc: netdev
mac_pton() parses MAC address in form XX:XX:XX:XX:XX:XX and
only in that form.
mac_pton() doesn't dirty result until it's sure string representation is valid.
mac_pton() doesn't care about characters _after_ last octet,
it's up to caller to deal with it.
mac_pton() diverges from 0/-E return value convention.
Target usage:
if (!mac_pton(str, whatever->mac))
return -EINVAL;
/* ->mac being u8 [ETH_ALEN] is filled at this point. */
/* optionally check str[3 * ETH_ALEN - 1] for termination */
Use mac_pton() in pktgen and netconsole for start.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
drivers/net/netconsole.c | 18 +++------------
include/linux/if_ether.h | 1
net/core/netpoll.c | 26 -----------------------
net/core/pktgen.c | 53 +++++++----------------------------------------
net/core/utils.c | 24 +++++++++++++++++++++
5 files changed, 38 insertions(+), 84 deletions(-)
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -473,23 +473,13 @@ static ssize_t store_remote_mac(struct netconsole_target *nt,
return -EINVAL;
}
- for (i = 0; i < ETH_ALEN - 1; i++) {
- remote_mac[i] = simple_strtoul(p, &p, 16);
- if (*p != ':')
- goto invalid;
- p++;
- }
- remote_mac[ETH_ALEN - 1] = simple_strtoul(p, &p, 16);
- if (*p && (*p != '\n'))
- goto invalid;
-
+ if (!mac_pton(buf, remote_mac))
+ return -EINVAL;
+ if (buf[3 * ETH_ALEN - 1] && buf[3 * ETH_ALEN - 1] != '\n')
+ return -EINVAL;
memcpy(nt->np.remote_mac, remote_mac, ETH_ALEN);
return strnlen(buf, count);
-
-invalid:
- printk(KERN_ERR "netconsole: invalid input\n");
- return -EINVAL;
}
/*
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -136,6 +136,7 @@ int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
extern struct ctl_table ether_table[];
#endif
+int mac_pton(const char *s, u8 *mac);
extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
#endif
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -698,32 +698,8 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
if (*cur != 0) {
/* MAC address */
- if ((delim = strchr(cur, ':')) == NULL)
+ if (!mac_pton(cur, np->remote_mac))
goto parse_failed;
- *delim = 0;
- np->remote_mac[0] = simple_strtol(cur, NULL, 16);
- cur = delim + 1;
- if ((delim = strchr(cur, ':')) == NULL)
- goto parse_failed;
- *delim = 0;
- np->remote_mac[1] = simple_strtol(cur, NULL, 16);
- cur = delim + 1;
- if ((delim = strchr(cur, ':')) == NULL)
- goto parse_failed;
- *delim = 0;
- np->remote_mac[2] = simple_strtol(cur, NULL, 16);
- cur = delim + 1;
- if ((delim = strchr(cur, ':')) == NULL)
- goto parse_failed;
- *delim = 0;
- np->remote_mac[3] = simple_strtol(cur, NULL, 16);
- cur = delim + 1;
- if ((delim = strchr(cur, ':')) == NULL)
- goto parse_failed;
- *delim = 0;
- np->remote_mac[4] = simple_strtol(cur, NULL, 16);
- cur = delim + 1;
- np->remote_mac[5] = simple_strtol(cur, NULL, 16);
}
netpoll_print_options(np);
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1420,11 +1420,6 @@ static ssize_t pktgen_if_write(struct file *file,
return count;
}
if (!strcmp(name, "dst_mac")) {
- char *v = valstr;
- unsigned char old_dmac[ETH_ALEN];
- unsigned char *m = pkt_dev->dst_mac;
- memcpy(old_dmac, pkt_dev->dst_mac, ETH_ALEN);
-
len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
if (len < 0)
return len;
@@ -1432,35 +1427,16 @@ static ssize_t pktgen_if_write(struct file *file,
memset(valstr, 0, sizeof(valstr));
if (copy_from_user(valstr, &user_buffer[i], len))
return -EFAULT;
- i += len;
-
- for (*m = 0; *v && m < pkt_dev->dst_mac + 6; v++) {
- int value;
-
- value = hex_to_bin(*v);
- if (value >= 0)
- *m = *m * 16 + value;
-
- if (*v == ':') {
- m++;
- *m = 0;
- }
- }
+ if (!mac_pton(valstr, pkt_dev->dst_mac))
+ return -EINVAL;
/* Set up Dest MAC */
- if (compare_ether_addr(old_dmac, pkt_dev->dst_mac))
- memcpy(&(pkt_dev->hh[0]), pkt_dev->dst_mac, ETH_ALEN);
+ memcpy(&pkt_dev->hh[0], pkt_dev->dst_mac, ETH_ALEN);
- sprintf(pg_result, "OK: dstmac");
+ sprintf(pg_result, "OK: dstmac %pM", pkt_dev->dst_mac);
return count;
}
if (!strcmp(name, "src_mac")) {
- char *v = valstr;
- unsigned char old_smac[ETH_ALEN];
- unsigned char *m = pkt_dev->src_mac;
-
- memcpy(old_smac, pkt_dev->src_mac, ETH_ALEN);
-
len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
if (len < 0)
return len;
@@ -1468,26 +1444,13 @@ static ssize_t pktgen_if_write(struct file *file,
memset(valstr, 0, sizeof(valstr));
if (copy_from_user(valstr, &user_buffer[i], len))
return -EFAULT;
- i += len;
-
- for (*m = 0; *v && m < pkt_dev->src_mac + 6; v++) {
- int value;
-
- value = hex_to_bin(*v);
- if (value >= 0)
- *m = *m * 16 + value;
-
- if (*v == ':') {
- m++;
- *m = 0;
- }
- }
+ if (!mac_pton(valstr, pkt_dev->src_mac))
+ return -EINVAL;
/* Set up Src MAC */
- if (compare_ether_addr(old_smac, pkt_dev->src_mac))
- memcpy(&(pkt_dev->hh[6]), pkt_dev->src_mac, ETH_ALEN);
+ memcpy(&pkt_dev->hh[6], pkt_dev->src_mac, ETH_ALEN);
- sprintf(pg_result, "OK: srcmac");
+ sprintf(pg_result, "OK: srcmac %pM", pkt_dev->src_mac);
return count;
}
--- a/net/core/utils.c
+++ b/net/core/utils.c
@@ -296,3 +296,27 @@ void inet_proto_csum_replace4(__sum16 *sum, struct sk_buff *skb,
csum_unfold(*sum)));
}
EXPORT_SYMBOL(inet_proto_csum_replace4);
+
+int mac_pton(const char *s, u8 *mac)
+{
+ int i;
+
+ /* XX:XX:XX:XX:XX:XX */
+ if (strlen(s) < 3 * ETH_ALEN - 1)
+ return 0;
+
+ /* Don't half dirty result. */
+ for (i = 0; i < ETH_ALEN; i++) {
+ if (!strchr("0123456789abcdefABCDEF", s[i * 3]))
+ return 0;
+ if (!strchr("0123456789abcdefABCDEF", s[i * 3 + 1]))
+ return 0;
+ if (i != ETH_ALEN - 1 && s[i * 3 + 2] != ':')
+ return 0;
+ }
+ for (i = 0; i < ETH_ALEN; i++) {
+ mac[i] = (hex_to_bin(s[i * 3]) << 4) | hex_to_bin(s[i * 3 + 1]);
+ }
+ return 1;
+}
+EXPORT_SYMBOL(mac_pton);
^ permalink raw reply
* Re: linux-next: manual merge of the rcu tree with the net tree
From: Paul E. McKenney @ 2011-05-04 6:05 UTC (permalink / raw)
To: Stephen Rothwell
Cc: linux-next, linux-kernel, Eric Dumazet, David Miller, netdev
In-Reply-To: <20110503142419.2b7d5e23.sfr@canb.auug.org.au>
On Tue, May 03, 2011 at 02:24:19PM +1000, Stephen Rothwell wrote:
> Hi Paul,
>
> Today's linux-next merge of the rcu tree got a conflict in
> net/core/filter.c between commit 0a14842f5a3c ("net: filter: Just In Time
> compiler for x86-64") from the net tree and commit 10cde158c259
> ("net,rcu: convert call_rcu(sk_filter_release_rcu) to kfree_rcu") from
> the rcu tree.
>
> The former adds another operation into sk_filter_release_rcu(), so I have
> effectively reverted the rcu tree change for now (by applying the patch
> below as a merge fixup).
Thank you, Stephen!
Looks like I need to get my changes forward-ported to the latest -rc...
Thanx, Paul
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 3 May 2011 14:06:50 +1000
> Subject: [PATCH] Revert "net,rcu: convert call_rcu(sk_filter_release_rcu) to
> kfree_rcu"
>
> This reverts commit 10cde158c2591422a2b32a2f560f406b8e69bee6.
> ---
> include/net/sock.h | 4 +++-
> net/core/filter.c | 13 +++++++++++++
> 2 files changed, 16 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 1a2f255..f2046e4 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1180,6 +1180,8 @@ extern void sk_common_release(struct sock *sk);
> /* Initialise core socket variables */
> extern void sock_init_data(struct socket *sock, struct sock *sk);
>
> +extern void sk_filter_release_rcu(struct rcu_head *rcu);
> +
> /**
> * sk_filter_release - release a socket filter
> * @fp: filter to remove
> @@ -1190,7 +1192,7 @@ extern void sock_init_data(struct socket *sock, struct sock *sk);
> static inline void sk_filter_release(struct sk_filter *fp)
> {
> if (atomic_dec_and_test(&fp->refcnt))
> - kfree_rcu(fp, rcu);
> + call_rcu(&fp->rcu, sk_filter_release_rcu);
> }
>
> static inline void sk_filter_uncharge(struct sock *sk, struct sk_filter *fp)
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 66d403d..0eb8c44 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -572,6 +572,19 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
> EXPORT_SYMBOL(sk_chk_filter);
>
> /**
> + * sk_filter_release_rcu - Release a socket filter by rcu_head
> + * @rcu: rcu_head that contains the sk_filter to free
> + */
> +void sk_filter_release_rcu(struct rcu_head *rcu)
> +{
> + struct sk_filter *fp = container_of(rcu, struct sk_filter, rcu);
> +
> + bpf_jit_free(fp);
> + kfree(fp);
> +}
> +EXPORT_SYMBOL(sk_filter_release_rcu);
> +
> +/**
> * sk_attach_filter - attach a socket filter
> * @fprog: the filter program
> * @sk: the socket to use
> --
> 1.7.4.4
>
^ permalink raw reply
* Re: [PATCH] usbnet: Transfer of maintainership
From: Richard Cochran @ 2011-05-04 5:45 UTC (permalink / raw)
To: Oliver Neukum
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
USB list
In-Reply-To: <201104291419.04498.oneukum-l3A5Bk7waGM@public.gmane.org>
On Fri, Apr 29, 2011 at 02:19:04PM +0200, Oliver Neukum wrote:
> USB "USBNET" DRIVER FRAMEWORK
> -M: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> +M: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
Oliver,
We have been looking at usbnet and a have question.
Usbnet doesn't use either phylib or napi, but I think the reason is
probably purely historical. Is there a technical reason why phylib or
napi won't work with usbnet devices?
If not, I would like to convert them in the near future.
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ethtool: Support to take FW dump
From: Anirban Chakraborty @ 2011-05-04 5:22 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <1304471194.3203.5.camel@localhost>
On May 3, 2011, at 6:06 PM, Ben Hutchings wrote:
> <snip>
>
> I understand that the userland application will need to get the size
> that way. I'm saying that this code in the kernel should also get the
> size from the driver, so that a malicious application cannot make the
> kernel allocate an excessively large buffer.
Makes sense. Will do so.
>
> Also, you'll need to submit your implementation along with this, as
> David won't accept an extension to the API without a driver that
> implements it.
I have implemented it in qlcnic driver. Will submit it along with the v2 patches.
Thanks.
-Anirban
^ permalink raw reply
* [net-next-2.6 PATCH] can: rename can_try_module_get to can_get_proto
From: Kurt Van Dijck @ 2011-05-04 4:42 UTC (permalink / raw)
To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
can: rename can_try_module_get to can_get_proto
can_try_module_get does return a struct can_proto.
The name explains what is done in so much detail that a caller
may not notice that a struct can_proto is locked/unlocked.
Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 5b52762..094fc53 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -115,7 +115,7 @@ static void can_sock_destruct(struct sock *sk)
skb_queue_purge(&sk->sk_receive_queue);
}
-static const struct can_proto *can_try_module_get(int protocol)
+static const struct can_proto *can_get_proto(int protocol)
{
const struct can_proto *cp;
@@ -128,6 +128,11 @@ static const struct can_proto *can_try_module_get(int protocol)
return cp;
}
+static inline void can_put_proto(const struct can_proto *cp)
+{
+ module_put(cp->prot->owner);
+}
+
static int can_create(struct net *net, struct socket *sock, int protocol,
int kern)
{
@@ -143,7 +148,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
if (!net_eq(net, &init_net))
return -EAFNOSUPPORT;
- cp = can_try_module_get(protocol);
+ cp = can_get_proto(protocol);
#ifdef CONFIG_MODULES
if (!cp) {
@@ -160,7 +165,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
printk(KERN_ERR "can: request_module "
"(can-proto-%d) failed.\n", protocol);
- cp = can_try_module_get(protocol);
+ cp = can_get_proto(protocol);
}
#endif
@@ -195,7 +200,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
}
errout:
- module_put(cp->prot->owner);
+ can_put_proto(cp);
return err;
}
^ permalink raw reply related
* [net-next-2.6 PATCH] can: make struct can_proto const
From: Kurt Van Dijck @ 2011-05-04 4:40 UTC (permalink / raw)
To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
commit 53914b67993c724cec585863755c9ebc8446e83b had the
same message. That commit did put everything in place but
did not make can_proto const itself.
Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index 6f70a6d..5ce6b5d 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -44,8 +44,8 @@ struct can_proto {
/* function prototypes for the CAN networklayer core (af_can.c) */
-extern int can_proto_register(struct can_proto *cp);
-extern void can_proto_unregister(struct can_proto *cp);
+extern int can_proto_register(const struct can_proto *cp);
+extern void can_proto_unregister(const struct can_proto *cp);
extern int can_rx_register(struct net_device *dev, canid_t can_id,
canid_t mask,
diff --git a/net/can/af_can.c b/net/can/af_can.c
index a8dcaa4..5b52762 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -84,7 +84,7 @@ static DEFINE_SPINLOCK(can_rcvlists_lock);
static struct kmem_cache *rcv_cache __read_mostly;
/* table of registered CAN protocols */
-static struct can_proto *proto_tab[CAN_NPROTO] __read_mostly;
+static const struct can_proto *proto_tab[CAN_NPROTO] __read_mostly;
static DEFINE_MUTEX(proto_tab_lock);
struct timer_list can_stattimer; /* timer for statistics update */
@@ -115,9 +115,9 @@ static void can_sock_destruct(struct sock *sk)
skb_queue_purge(&sk->sk_receive_queue);
}
-static struct can_proto *can_try_module_get(int protocol)
+static const struct can_proto *can_try_module_get(int protocol)
{
- struct can_proto *cp;
+ const struct can_proto *cp;
rcu_read_lock();
cp = rcu_dereference(proto_tab[protocol]);
@@ -132,7 +132,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
int kern)
{
struct sock *sk;
- struct can_proto *cp;
+ const struct can_proto *cp;
int err = 0;
sock->state = SS_UNCONNECTED;
@@ -691,7 +691,7 @@ drop:
* -EBUSY protocol already in use
* -ENOBUF if proto_register() fails
*/
-int can_proto_register(struct can_proto *cp)
+int can_proto_register(const struct can_proto *cp)
{
int proto = cp->protocol;
int err = 0;
@@ -728,7 +728,7 @@ EXPORT_SYMBOL(can_proto_register);
* can_proto_unregister - unregister CAN transport protocol
* @cp: pointer to CAN protocol structure
*/
-void can_proto_unregister(struct can_proto *cp)
+void can_proto_unregister(const struct can_proto *cp)
{
int proto = cp->protocol;
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8a6a05e..cced806 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1601,7 +1601,7 @@ static struct proto bcm_proto __read_mostly = {
.init = bcm_init,
};
-static struct can_proto bcm_can_proto __read_mostly = {
+static const struct can_proto bcm_can_proto = {
.type = SOCK_DGRAM,
.protocol = CAN_BCM,
.ops = &bcm_ops,
diff --git a/net/can/raw.c b/net/can/raw.c
index 0eb39a7..dea99a6 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -774,7 +774,7 @@ static struct proto raw_proto __read_mostly = {
.init = raw_init,
};
-static struct can_proto raw_can_proto __read_mostly = {
+static const struct can_proto raw_can_proto = {
.type = SOCK_RAW,
.protocol = CAN_RAW,
.ops = &raw_ops,
^ permalink raw reply related
* Re: [PATCH v4 2/5] can: add rtnetlink support
From: Kurt Van Dijck @ 2011-05-04 4:20 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4DBD9586.7080908-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
> Besides some minor adaptions to support the SVN i left out some infrastructure
> changes you made in patch 2/5 to constify this ...
>
> > -static struct can_proto *proto_tab[CAN_NPROTO] __read_mostly;
> > +static const struct can_proto *proto_tab[CAN_NPROTO] __read_mostly;
>
> and this ...
>
> > -static struct can_proto *can_try_module_get(int protocol)
> > +static const struct can_proto *can_try_module_get(int protocol)
>
> (..)
>
> > +static inline void can_put_proto(const struct can_proto *cp)
> > +{
> > + module_put(cp->prot->owner);
> > +}
>
> These infrastructure changes did not really belong to the rtnetlink support.
I see. I see I've made a mistake in the patch set here.
I'll go for a seperate 'make it const' patch.
Thanks,
Kurt
^ permalink raw reply
* Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface
From: Alexander Duyck @ 2011-05-04 3:10 UTC (permalink / raw)
To: Dimitris Michailidis
Cc: Alexander Duyck, Ben Hutchings, davem@davemloft.net,
Kirsher, Jeffrey T, netdev@vger.kernel.org
In-Reply-To: <4DC0AD7B.7070009@chelsio.com>
On Tue, May 3, 2011 at 6:35 PM, Dimitris Michailidis <dm@chelsio.com> wrote:
> On 05/03/2011 05:29 PM, Alexander Duyck wrote:
>> The thing to keep in mind is that the index doesn't have to be a hardware
>> index. In ixgbe we have a field in the hardware which is meant to just be a
>> unique software index and that is what I am using as the location field for
>> our filters. All the location information in the rules really provides is a
>> logical way of tracking rules. It doesn't necessarily have to represent the
>> physical location of the rule in hardware.
>
> I appreciate the intent but there are couple problems.
> a) ethtool.h documents location as
>
> * @location: Index of filter in hardware table
>
> i.e., physical location. But we could change that.
I will probably want to change that. The fact is as I recall even niu
was using a hash in addition to the location index. As such it isn't
really the true location in the hardware since there is a second piece
to determining the actual location in hardware.
> b) for TCAMs physical location is important and if ethtool offers to provide
> only a logical index and massages the original user input to do so where
> will a driver get the physical location it ultimately needs? For a device
> with physical indices a multiple of 4 the logical index ethtool picks will
> very frequently be illegal as physical location. E.g., if location 1 is
> available ethtool will keep selecting it and the driver will need to deal
> with these requests without the benefit of knowing what the user really
> asked for.
>
> Another problem with ethtool selecting locations is it assumes it's the sole
> allocator (I think there's a comment in the code pointing this out). But
> this isn't a valid assumption, e.g., HW RFS comes to mind as another
> allocator.
The other thing to keep in mind is that this doesn't preclude the
option of adding an ethtool command at some point in the future for
handling the determination of filter location. The fact is all that
would need to be done is to add an extra ioctl call to determine the
location based on the filter and if that returns op not supported we
fall back to the current rule manager.
The way I have things setup now provides a good foundation in
user-space for managing the rules. I fully admit it doesn't fit all
solutions, and I welcome follow-on patches to add extra functionality,
but at this time I really would prefer avoiding adding extra
functionality for yet to be implemented features in device drivers.
The ntuple display functionality provides a good example of why I
would prefer to avoid it. Everything looked like it should have
worked when get_rx_ntuple was implemented in the device drivers, but
as soon as I implemented a get_rx_ntuple for ixgbe I quickly
discovered it didn't work and as such I am now stuck moving everything
over to network flow classifier for ixgbe.
Thanks,
Alex
^ permalink raw reply
* Re: [RFC v3 02/10] Revert "lsm: Remove the socket_post_accept() hook"
From: Tetsuo Handa @ 2011-05-04 2:28 UTC (permalink / raw)
To: paul.moore, sam
Cc: linux-security-module, linux-kernel, netdev, netfilter-devel,
hadi, kaber, zbr, root
In-Reply-To: <201105031802.34724.paul.moore@hp.com>
Paul Moore wrote:
> On Tuesday, May 03, 2011 10:24:15 AM Samir Bellabes wrote:
> > snet needs to reintroduce this hook, as it was designed to be: a hook for
> > updating security informations on objects.
>
> Looking at this and 5/10 again, it seems that you should be able to do what
> you need with the sock_graft() hook. Am I missing something?
>
> My apologies if we've already discussed this approach previously ...
static void snet_socket_post_accept(struct socket *sock, struct socket *newsock)
{
static void snet_do_send_event(struct snet_info *info)
{
int snet_nl_send_event(struct snet_info *info)
{
skb_rsp = genlmsg_new(size, GFP_KERNEL);
genlmsg_unicast()
}
}
}
First problem with using snet_do_send_event() from security_sock_graft() is
that we have to use GFP_ATOMIC rather than GFP_KERNEL because we are inside
write_lock_bh()/write_unlock_bh().
static inline int genlmsg_unicast(struct net *net, struct sk_buff *skb, u32 pid)
{
static inline int nlmsg_unicast(struct sock *sk, struct sk_buff *skb, u32 pid)
{
int netlink_unicast(struct sock *ssk, struct sk_buff *skb,
u32 pid, MSG_DONTWAIT)
{
int netlink_attachskb(struct sock *sk, struct sk_buff *skb,
long *timeo, struct sock *ssk)
{
if (!*timeo) {
return -EAGAIN;
}
}
}
}
Second problem is that genlmsg_unicast() might return -EAGAIN because we can't
sleep inside write_lock_bh()/write_unlock_bh().
Third problem (though independent with security_sock_graft()) is that
snet_do_send_event() ignores snet_nl_send_event() failure.
^ permalink raw reply
* Re: [PATCHv3 2/2] tg3: Allow ethtool to enable/disable loopback.
From: Matt Carlson @ 2011-05-04 1:52 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Matthew Carlson, David Miller, netdev, Michael Chan,
Ben Hutchings, Micha? Miros?aw, Tom Herbert
In-Reply-To: <1304471935-402-3-git-send-email-maheshb@google.com>
On Tue, May 03, 2011 at 06:18:55PM -0700, Mahesh Bandewar wrote:
> + /*
> + * Add loopback capability only for a subset of devices that support
> + * MAC-LOOPBACK. Eventually this need to be enhanced to allow INT-PHY
> + * loopback for the remaining devices.
> + */
> + if (GET_ASIC_REV(tp->pci_chip_rev_id) != ASIC_REV_5780 ||
> + !tg3_flag(tp, CPMU_PRESENT))
> + /* Add the loopback capability */
> + dev->hw_features |= NETIF_F_LOOPBACK;
s/||/&&/
^ permalink raw reply
* Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface
From: Dimitris Michailidis @ 2011-05-04 1:35 UTC (permalink / raw)
To: Alexander Duyck
Cc: Ben Hutchings, davem@davemloft.net, Kirsher, Jeffrey T,
netdev@vger.kernel.org
In-Reply-To: <4DC09DE0.8070102@intel.com>
On 05/03/2011 05:29 PM, Alexander Duyck wrote:
> On 5/3/2011 4:34 PM, Ben Hutchings wrote:
>> On Tue, 2011-05-03 at 16:23 -0700, Dimitris Michailidis wrote:
>>> On 05/03/2011 09:12 AM, Alexander Duyck wrote:
>> [...]
>>>> +int rxclass_rule_ins(int fd, struct ifreq *ifr,
>>>> + struct ethtool_rx_flow_spec *fsp)
>>>> +{
>>>> + struct ethtool_rxnfc nfccmd;
>>>> + __u32 loc = fsp->location;
>>>> + int err;
>>>> +
>>>> + /*
>>>> + * if location is unspecified pull rules from device
>>>> + * and allocate a free rule for our use
>>>> + */
>>>> + if (loc == RX_CLS_LOC_UNSPEC) {
>>>> + /* init table of available rules */
>>>> + err = rmgr_init(fd, ifr);
>>>> + if (err< 0)
>>>> + return err;
>>>> +
>>>> + /* verify rule location */
>>>> + err = rmgr_add(fsp);
>>>> + if (err< 0)
>>>> + return err;
>>>> +
>>>> + /* cleanup table and free resources */
>>>> + rmgr_cleanup();
>>>> + }
>>>
>>> This logic where ethtool tries to select a filter slot when a user
>>> provides
>>> RX_CLS_LOC_UNSPEC does not work in general. It assumes that all
>>> slots are
>>> equal and a new filter can go into any available slot. But a device
>>> may have
>>> restrictions on where a filter may go that ethtool doesn't know.
>>
>> I agree. And if filter lookup is largely hash-based (as it is in
>> Solarflare hardware) the user will also find it very difficult to
>> specify the right location!
>
> The thing to keep in mind is that the index doesn't have to be a
> hardware index. In ixgbe we have a field in the hardware which is meant
> to just be a unique software index and that is what I am using as the
> location field for our filters. All the location information in the
> rules really provides is a logical way of tracking rules. It doesn't
> necessarily have to represent the physical location of the rule in
> hardware.
I appreciate the intent but there are couple problems.
a) ethtool.h documents location as
* @location: Index of filter in hardware table
i.e., physical location. But we could change that.
b) for TCAMs physical location is important and if ethtool offers to provide
only a logical index and massages the original user input to do so where
will a driver get the physical location it ultimately needs? For a device
with physical indices a multiple of 4 the logical index ethtool picks will
very frequently be illegal as physical location. E.g., if location 1 is
available ethtool will keep selecting it and the driver will need to deal
with these requests without the benefit of knowing what the user really
asked for.
Another problem with ethtool selecting locations is it assumes it's the sole
allocator (I think there's a comment in the code pointing this out). But
this isn't a valid assumption, e.g., HW RFS comes to mind as another allocator.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox