* Re: [net-next-2.6 PATCH v5 0/3] Introduce n-tuple ethtool support
From: Peter P Waskiewicz Jr @ 2010-02-11 4:18 UTC (permalink / raw)
To: David Miller; +Cc: Kirsher, Jeffrey T, netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <20100210.195445.190157100.davem@davemloft.net>
On Wed, 2010-02-10 at 19:54 -0800, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Wed, 10 Feb 2010 18:07:18 -0800
>
> > One more round of fixes, based on feedback from Patrick McHardy
> >
> > 1) Change the list count to an unsigned value
> > 2) Fix a memory leak
> > 3) Removed an unnecessary list traversal in the ethtool core
> > 4) Moved all list destruction to a helper function, allowing the driver
> > to control when it clears the list (aside from when free_netdev() kills
> > the cached list).
>
> All applied, thanks.
>
> Pj, can you look a shoring up the behavior of one more thing for me?
>
> When rules get added, it first installs the filter by calling into
> the driver.
>
> _Then_ is tries to add the software copy of the rule to the device
> list. This does an allocation which may fail.
>
> If it does fail, we return -ENOMEM but we left the device configured
> with the new rule.
>
> Probably better to do it something like:
>
> p = NULL;
> if (need_sw_rule_list)
> p = alloc_rule();
> if (!p)
> return -ENOMEM;
> }
>
> ret = op->install_rule();
> if (ret) {
> kfree(p);
> return ret;
> }
>
> if (need_sw_rule_list)
> sw_rule_insert(dev, p);
>
> You get the idea, we can do the kfree() unconditonally because kfree(NULL)
> is OK, etc.
No problem. I'll get a patch together asap for 2.6.34.
Also, probably for 2.6.35, I'm going to try and add a remove capability.
Cheers,
-PJ
^ permalink raw reply
* RE: [PATCH 1/3] A device for zero-copy based on KVM virtio-net.
From: Xin, Xiaohui @ 2010-02-11 5:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mingo@elte.hu, mst@redhat.com,
jdike@c2.user-mode-linux.org, Zhao Yu
In-Reply-To: <1265815071.3047.96.camel@edumazet-laptop>
Eric,
Thanks. I will look into that. But don't stop there.
Please comments more. :-)
Thanks
Xiaohui
-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Wednesday, February 10, 2010 11:18 PM
To: Xin, Xiaohui
Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org; mingo@elte.hu; mst@redhat.com; jdike@c2.user-mode-linux.org; Zhao Yu
Subject: Re: [PATCH 1/3] A device for zero-copy based on KVM virtio-net.
Le mercredi 10 février 2010 à 19:48 +0800, Xin Xiaohui a écrit :
> Add a device to utilize the vhost-net backend driver for
> copy-less data transfer between guest FE and host NIC.
> It pins the guest user space to the host memory and
> provides proto_ops as sendmsg/recvmsg to vhost-net.
>
> Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
> Signed-off-by: Zhao Yu <yzhao81@gmail.com>
> Sigend-off-by: Jeff Dike <jdike@c2.user-mode-linux.org>
> +static int page_ctor_attach(struct mp_struct *mp)
> +{
> + int rc;
> + struct page_ctor *ctor;
> + struct net_device *dev = mp->dev;
> +
> + rcu_read_lock();
> + if (rcu_dereference(mp->ctor)) {
> + rcu_read_unlock();
> + return -EBUSY;
> + }
> + rcu_read_unlock();
Strange read locking here, for an obvious writer role.
What do you really want to do ?
If writer are serialized by mp_mutex, you dont need this
recu_read_lock()/rcu_read_unlock() stuff.
> +
> + ctor = kzalloc(sizeof(*ctor), GFP_KERNEL);
> + if (!ctor)
> + return -ENOMEM;
> + rc = netdev_page_ctor_prep(dev, &ctor->ctor);
> + if (rc)
> + goto fail;
> +
> + ctor->cache = kmem_cache_create("skb_page_info",
> + sizeof(struct page_info), 0,
> + SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
SLAB_PANIC here means : crash whole system in case of error.
This is not what you want in a driver.
> +
> + if (!ctor->cache)
> + goto cache_fail;
> +
> + INIT_LIST_HEAD(&ctor->readq);
> + spin_lock_init(&ctor->read_lock);
> +
> + ctor->w_len = 0;
> + ctor->r_len = 0;
> +
> + dev_hold(dev);
> + ctor->dev = dev;
> + ctor->ctor.ctor = page_ctor;
> + ctor->ctor.sock = &mp->socket;
> + atomic_set(&ctor->refcnt, 1);
> +
> + rc = netdev_page_ctor_attach(dev, &ctor->ctor);
> + if (rc)
> + goto fail;
> +
> + /* locked by mp_mutex */
> + rcu_assign_pointer(mp->ctor, ctor);
> +
> + /* XXX:Need we do set_offload here ? */
> +
> + return 0;
> +
> +fail:
> + kmem_cache_destroy(ctor->cache);
> +cache_fail:
> + kfree(ctor);
> + dev_put(dev);
> +
> + return rc;
> +}
> +
> +
> +static inline void get_page_ctor(struct page_ctor *ctor)
> +{
> + atomic_inc(&ctor->refcnt);
> +}
> +
> +static inline void put_page_ctor(struct page_ctor *ctor)
> +{
> + if (atomic_dec_and_test(&ctor->refcnt))
> + kfree(ctor);
Are you sure a RCU grace period is not needed before freeing ?
> +
> +static int page_ctor_detach(struct mp_struct *mp)
> +{
> + struct page_ctor *ctor;
> + struct page_info *info;
> + int i;
> +
> + rcu_read_lock();
> + ctor = rcu_dereference(mp->ctor);
> + rcu_read_unlock();
Strange locking again here
> +
> + if (!ctor)
> + return -ENODEV;
> +
> + while ((info = info_dequeue(ctor))) {
> + for (i = 0; i < info->pnum; i++)
> + if (info->pages[i])
> + put_page(info->pages[i]);
> + kmem_cache_free(ctor->cache, info);
> + }
> + kmem_cache_destroy(ctor->cache);
> + netdev_page_ctor_detach(ctor->dev);
> + dev_put(ctor->dev);
> +
> + /* locked by mp_mutex */
> + rcu_assign_pointer(mp->ctor, NULL);
> + synchronize_rcu();
> +
> + put_page_ctor(ctor);
> +
> + return 0;
> +}
> +
> +/* For small user space buffers transmit, we don't need to call
> + * get_user_pages().
> + */
> +static struct page_info *alloc_small_page_info(struct page_ctor *ctor,
> + int total)
> +{
> + struct page_info *info = kmem_cache_alloc(ctor->cache, GFP_KERNEL);
kmem_cache_zalloc() ?
> +
> + if (!info)
> + return NULL;
> + memset(info, 0, sizeof(struct page_info));
> + memset(info->pages, 0, sizeof(info->pages));
redundant memset() whole structure already cleared one line above
> +
> + info->header = 0;
already cleared
> + info->total = total;
> + info->skb = NULL;
already cleared
>
> + info->user.dtor = page_dtor;
> + info->ctor = ctor;
> + info->flags = INFO_WRITE;
> + info->pnum = 0;
already cleared
>
> + return info;
> +}
> +
> +/* The main function to transform the guest user space address
> + * to host kernel address via get_user_pages(). Thus the hardware
> + * can do DMA directly to the user space address.
> + */
> +static struct page_info *alloc_page_info(struct page_ctor *ctor,
> + struct iovec *iov, int count, struct frag *frags,
> + int npages, int total)
> +{
> + int rc;
> + int i, j, n = 0;
> + int len;
> + unsigned long base;
> + struct page_info *info = kmem_cache_alloc(ctor->cache, GFP_KERNEL);
kmem_cache_zalloc() ?
>
> +
> + if (!info)
> + return NULL;
> + memset(info, 0, sizeof(struct page_info));
kmem_cache_zalloc() ?
> + memset(info->pages, 0, sizeof(info->pages));
already cleared
>
> +
> + down_read(¤t->mm->mmap_sem);
> + for (i = j = 0; i < count; i++) {
> + base = (unsigned long)iov[i].iov_base;
> + len = iov[i].iov_len;
> +
> + if (!len)
> + continue;
> + n = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
> +
> + rc = get_user_pages(current, current->mm, base, n,
> + npages ? 1 : 0, 0, &info->pages[j], NULL);
> + if (rc != n) {
> + up_read(¤t->mm->mmap_sem);
> + goto failed;
> + }
> +
> + while (n--) {
> + frags[j].offset = base & ~PAGE_MASK;
> + frags[j].size = min_t(int, len,
> + PAGE_SIZE - frags[j].offset);
> + len -= frags[j].size;
> + base += frags[j].size;
> + j++;
> + }
> + }
> + up_read(¤t->mm->mmap_sem);
> +
> +#ifdef CONFIG_HIGHMEM
> + if (npages && !(dev->features & NETIF_F_HIGHDMA)) {
> + for (i = 0; i < j; i++) {
> + if (PageHighMem(info->pages[i]))
> + goto failed;
> + }
> + }
> +#endif
> +
> + info->header = 0;
> + info->total = total;
> + info->skb = NULL;
> + info->user.dtor = page_dtor;
> + info->ctor = ctor;
> + info->pnum = j;
> +
> + if (!npages)
> + info->flags = INFO_WRITE;
> + if (info->flags == INFO_READ) {
> + info->user.start = (u8 *)(((unsigned long)
> + (pfn_to_kaddr(page_to_pfn(info->pages[0]))) +
> + frags[0].offset) - NET_IP_ALIGN - NET_SKB_PAD);
> + info->user.size = iov[0].iov_len + NET_IP_ALIGN + NET_SKB_PAD;
> + }
> + return info;
> +
> +failed:
> + for (i = 0; i < j; i++)
> + put_page(info->pages[i]);
> +
> + kmem_cache_free(ctor->cache, info);
> +
> + return NULL;
> +}
> +
> +struct page_ctor *mp_rcu_get_ctor(struct page_ctor *ctor)
> +{
> + struct page_ctor *_ctor = NULL;
> +
> + rcu_read_lock();
> + _ctor = rcu_dereference(ctor);
> + rcu_read_unlock();
strange locking. After rcu_read_unlock() you have no guarantee _ctor
points to something not freed.
> +
> + if (!_ctor) {
> + DBG(KERN_INFO "Device %s cannot do mediate passthru.\n",
> + ctor->dev->name);
> + return NULL;
> + }
> + if (_ctor)
redundant test
> + get_page_ctor(_ctor);
> + return _ctor;
> +}
> +
I stopped my review at this point. Please check your RCU usages. It is
not sufficient to hold rcu read lock just to fetch the pointer, you also
must hold the lock while using the object itself, or get a reference on
object before release RCU lock, to make sure object wont disappear under
you...
for example :
rcu_read_lock();
ptr = rcu_dereference(...);
if (ptr)
atomic_inc(&ptr->refcnt);
rcu_read_unlock();
^ permalink raw reply
* Re: [net-next-2.6 PATCH v3 4/5] rtnetlink: Add VF config code to rtnetlink
From: Patrick McHardy @ 2010-02-11 6:07 UTC (permalink / raw)
To: Williams, Mitch A
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <EA929A9653AAE14F841771FB1DE5A1365FE3CBE343@rrsmsx501.amr.corp.intel.com>
Williams, Mitch A wrote:
>> From: Patrick McHardy [mailto:kaber@trash.net]
> [snip]
>> We usually encapsulate lists of the same attribute type in another
>> top-level attribute. Check out the IFLA_VLAN_*_QOS attributes for
>> an example.
>>
>> The interface should also be symetrical, IOW you should dump the
>> same attributes used in the userspace->kernel direction instead
>> of a combined "info" attribute.
>
> Sheesh, Patrick, where were you three months ago when I first
> posted this stuff? It would have helped a lot if I heard from
> you back then. We've had at least five internal review cycles
> here and nobody caught this, mostly because nobody understands
> it.
Well .. sorry :)
> That being said, I'll take another look at the NLA_NESTED stuff
> and see what I can figure out. Do you know of any place (outside
> of the code) where this is documented? It's particularly
> difficult to follow this code.
No, but its quite simple. For dumping attributes, you create
a top-level nested attribute, then add all inner attributes
and "end" the top-level attribute, which fills in the entire
length. In terms of the kernel netlink helpers:
nest = nla_nest_start(skb, IFLA_VF);
if (nest == NULL)
goto nla_put_failure;
for (...) {
<get info>
NLA_PUT(skb, IFLA_VF_INFO, &info);
}
nla_nest_end(skb, nest);
For parsing withing the kernel, you use nla_for_each_nested()
to iterate over the encapsulated attributes and (unless the
inner attributes are nested themselves) nla_data() to get
at the payload:
nla_for_each_nested(attr, nla[IFLA_VF_INFO], rem) {
if (nla_type(attr) != MY_TYPE)
goto err_inval;
info = nla_data(attr);
<do something with info>
}
You can also put nested attributes into the list, in that case
you'd use nla_parse_nested(attr, ...) instead of nla_data().
> I see your point about symmetrical interfaces, but I'm not sure
> it's the best thing here. We want the user to be able to set these
> attributes independently, without blowing away any other settings.
> If we put all three settings together into one data structure,
> the code flow will end up being much more complicated.
Using a symetrical interface doesn't necessarily prevent incremental
configuration. Please see below.
> I'd prefer to leave the data structures as they are, and switch
> to using nested attributes for the status reporting part, i.e.
> what happens when you type 'ip link show'. Would this work for you?
The way rtnetlink is currently built the only difference between
configuration requests from userspace and notifications from the
kernel is the NLM_F_REQUEST bit. This is a useful property because
you can re-create objects in the kernel simply be replaying a
notification as request. Its also necessary to be able to use
the same parsing functions for notifications and error messages
in userspace (error messages have the original request appended).
What should be relatively easy to do is to use lists of (combined
or non-combined) attributes in both directions. In the userspace->
kernel direction all you need to change is to encapsulate your
new attributes and walk through the list. In the kernel->userspace
direction you can use the combined struct within the kernel if
thats more useful and translate it into individual attributes when
filling the netlink message.
^ permalink raw reply
* Re: [net-next PATCH v3 3/3] net: reserve ports for applications using fixed port numbers
From: Eric Dumazet @ 2010-02-11 6:12 UTC (permalink / raw)
To: Octavian Purdila; +Cc: netdev
In-Reply-To: <201002110453.35242.opurdila@ixiacom.com>
Octavian, please resubmit all patches to lkml, netdev, David, because
patches 1 & 2 are changing kernel core services.
However, I'll take some time in a couple of hours to review them.
Le jeudi 11 février 2010 à 04:53 +0200, Octavian Purdila a écrit :
> This patch introduces /proc/sys/net/ipv4/ip_local_reserved_ports
> (bitmap type) which allows users to reserve ports for third-party
> applications.
>
> The reserved ports will not be used by automatic port assignments
> (e.g. when calling connect() or bind() with port number 0). Explicit
> port allocation behavior is unchanged.
>
> +extern unsigned long sysctl_local_reserved_ports[65536/8/sizeof(unsigned long)];
I am sure we have a special macro for this.
extern DECLARE_BITMAP(reserved_ports, 65536);
> +unsigned long sysctl_local_reserved_ports[65536/BITS_PER_LONG];
> +
Same point here, plus I am not sure adding 8192 bytes in BSS zone is a
problem nowadays. (It was ten years ago for some arches)
^ permalink raw reply
* Re: [net-next PATCH v3 3/3] net: reserve ports for applications using fixed port numbers
From: Eric Dumazet @ 2010-02-11 6:14 UTC (permalink / raw)
To: Octavian Purdila; +Cc: netdev
In-Reply-To: <1265868768.3061.15.camel@edumazet-laptop>
Le jeudi 11 février 2010 à 07:12 +0100, Eric Dumazet a écrit :
> Octavian, please resubmit all patches to lkml, netdev, David, because
> patches 1 & 2 are changing kernel core services.
>
Ooops, I just saw your second submission, please ignore my comment :)
^ permalink raw reply
* Re: [net-next-2.6, v4 1/3] ethtool: Introduce n-tuple filter programming support
From: Patrick McHardy @ 2010-02-11 6:16 UTC (permalink / raw)
To: Waskiewicz Jr, Peter P
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <Pine.WNT.4.64.1002101533340.58956@ppwaskie-MOBL2.amr.corp.intel.com>
Waskiewicz Jr, Peter P wrote:
> On Wed, 10 Feb 2010, Patrick McHardy wrote:
>
> Thanks for the review Patrick. Comments inline.
>
> -PJ
>
>> Jeff Kirsher wrote:
>>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>>> index ef4a2d8..4e9ef85 100644
>>> --- a/include/linux/ethtool.h
>>> +++ b/include/linux/ethtool.h
>>> +struct ethtool_rx_ntuple_list {
>>> +#define ETHTOOL_MAX_NTUPLE_LIST_ENTRY 1024
>>> +#define ETHTOOL_MAX_NTUPLE_STRING_PER_ENTRY 14
>>> + struct list_head list;
>>> + int count;
>> unsigned int seems more appropriate.
>
> Really? It's a count of the number of cached filters. Is it just so we
> don't overflow? I don't have any strong preference, so I can update this.
Mainly because I don't think we can have a negative number
of filters :)
>>> u32 ethtool_op_get_flags(struct net_device *dev)
>>> {
>>> @@ -139,6 +139,11 @@ int ethtool_op_set_flags(struct net_device *dev, u32 data)
>>> else
>>> dev->features &= ~NETIF_F_LRO;
>>>
>>> + if (data & ETH_FLAG_NTUPLE)
>>> + dev->features |= NETIF_F_NTUPLE;
>>> + else
>>> + dev->features &= ~NETIF_F_NTUPLE;
>> Shouldn't this check for the real capabilities of the device first?
>
> The userspace side does before it calls the ioctl. It will abort with a
> -EOPNOTSUPP (just tested with igb - properly aborted).
I think this check belongs into the kernel. You already have these
two checks in ethtool_set_rx_ntuple():
> + if (!ops->set_rx_ntuple)
> + return -EOPNOTSUPP;
> +
> + if (!(dev->features & NETIF_F_NTUPLE))
> + return -EINVAL;
Moving the check for ops->set_rx_ntuple to ethtool_op_set_flags()
should be enough.
>>> +static int ethtool_get_rx_ntuple(struct net_device *dev, void __user *useraddr)
>>> +{
>>> + struct ethtool_gstrings gstrings;
>>> + const struct ethtool_ops *ops = dev->ethtool_ops;
>>> + struct ethtool_rx_ntuple_flow_spec_container *fsc;
>>> + u8 *data;
>>> + char *p;
>>> + int ret, i, num_strings = 0;
>>> +
>>> + if (!ops->get_sset_count)
>>> + return -EOPNOTSUPP;
>>> +
>>> + if (copy_from_user(&gstrings, useraddr, sizeof(gstrings)))
>>> + return -EFAULT;
>>> +
>>> + ret = ops->get_sset_count(dev, gstrings.string_set);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + gstrings.len = ret;
>>> +
>>> + data = kmalloc(gstrings.len * ETH_GSTRING_LEN, GFP_USER);
>>> + if (!data)
>>> + return -ENOMEM;
>>> +
>>> + if (ops->get_rx_ntuple) {
>>> + /* driver-specific filter grab */
>>> + ret = ops->get_rx_ntuple(dev, gstrings.string_set, data);
>>> + goto copy;
>>> + }
>>> +
>>> + /* default ethtool filter grab */
>>> + i = 0;
>>> + p = (char *)data;
>>> + list_for_each_entry(fsc, &dev->ethtool_ntuple_list.list, list) {
>>> + sprintf(p, "Filter %d:\n", i);
>> Providing a textual representation from within the kernel doesn't
>> seem like a good interface to me. If userspace wants to do anything
>> but simply display them, it will have to parse them again. Additionally
>> it seems a driver providing a ->get_rx_ntuple() callback would have
>> to duplicate the entire conversion code, which is error prone.
>
> The goal was to give a generic way to dump what was programmed, if an
> underlying driver didn't want to implement the ->get_rx_ntuple()
> operation. The two ways I could think of doing it was dump the list the
> way I did, and provide a strings blob to ethtool (like stats), or try and
> package the structs into a list, copy that to userspace, and let ethtool
> generate the blobs.
>
> I agree that an underlying driver will have much of the same in terms of
> what it generates, but it will not be restricted to how it stores the
> items. In other words, if ixgbe wanted to retrieve all 8192 filters, we
> could avoid the caching altogether, and pull directly from HW when the
> call is made from ethtool. One way or another, there's going to be a big
> amount of copied data from kernel space to user space. This was the
> approach I thougt would be the most useful without defining a kernel to
> userspace chain of flow spec structs.
My main concern is that its hard for userspace to do anything with
this data except print it. By using a binary representation the
kernel code should get simpler and less prone to potential
inconsistencies within drivers and make it more useful to userspace
at the same time.
^ permalink raw reply
* net-2.6 [PATCH 0/2] udp/dccp: MSG_TRUNC
From: Gerrit Renker @ 2010-02-11 6:26 UTC (permalink / raw)
To: davem; +Cc: dccp, netdev
In-Reply-To: <udp_dccp_msg_trunc>
Hi Dave,
I have here two datagram-API patches, both submitted for net-2.6:
Patch #1: Resolves missing MSG_TRUNC support. Since DCCP is datagram-based,
the user-space API is broken/incomplete without.
Patch #2: UDP recvmsg() cleanup, remove redundant variable.
^ permalink raw reply
* [PATCH 1/2] dccp: support for passing MSG_TRUNC
From: Gerrit Renker @ 2010-02-11 6:26 UTC (permalink / raw)
To: davem; +Cc: dccp, netdev, Gerrit Renker
In-Reply-To: <1265869579-5829-1-git-send-email-gerrit@erg.abdn.ac.uk>
DCCP is datagram-oriented but lacks UDP's support for MSG_TRUNC as defined in
recvmsg(2)/recv(2). Hence the following 'Hello world\0' receiver
len = recv(fd, buf, 10, MSG_PEEK | MSG_TRUNC);
wrongly (always) returns 10, while in UDP it returns 12 as expected.
This patch adds the missing MSG_TRUNC support to recvmsg().
Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
net/dccp/proto.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -835,6 +835,8 @@ verify_sock_status:
len = -EFAULT;
break;
}
+ if (flags & MSG_TRUNC)
+ len = skb->len;
found_fin_ok:
if (!(flags & MSG_PEEK))
sk_eat_skb(sk, skb, 0);
^ permalink raw reply
* [PATCH 2/2] udp: remove redundant variable
From: Gerrit Renker @ 2010-02-11 6:26 UTC (permalink / raw)
To: davem; +Cc: dccp, netdev, Gerrit Renker
In-Reply-To: <1265869579-5829-2-git-send-email-gerrit@erg.abdn.ac.uk>
The variable 'copied' is used in udp_recvmsg() to emphasize that the passed
'len' is adjusted to fit the actual datagram length. But the same can be
done by adjusting 'len' directly. This patch thus removes the indirection.
Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
net/ipv4/udp.c | 15 +++++++--------
net/ipv6/udp.c | 15 +++++++--------
2 files changed, 14 insertions(+), 16 deletions(-)
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1117,7 +1117,7 @@ int udp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
struct inet_sock *inet = inet_sk(sk);
struct sockaddr_in *sin = (struct sockaddr_in *)msg->msg_name;
struct sk_buff *skb;
- unsigned int ulen, copied;
+ unsigned int ulen;
int peeked;
int err;
int is_udplite = IS_UDPLITE(sk);
@@ -1138,10 +1138,9 @@ try_again:
goto out;
ulen = skb->len - sizeof(struct udphdr);
- copied = len;
- if (copied > ulen)
- copied = ulen;
- else if (copied < ulen)
+ if (len > ulen)
+ len = ulen;
+ else if (len < ulen)
msg->msg_flags |= MSG_TRUNC;
/*
@@ -1150,14 +1149,14 @@ try_again:
* coverage checksum (UDP-Lite), do it before the copy.
*/
- if (copied < ulen || UDP_SKB_CB(skb)->partial_cov) {
+ if (len < ulen || UDP_SKB_CB(skb)->partial_cov) {
if (udp_lib_checksum_complete(skb))
goto csum_copy_err;
}
if (skb_csum_unnecessary(skb))
err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr),
- msg->msg_iov, copied);
+ msg->msg_iov, len);
else {
err = skb_copy_and_csum_datagram_iovec(skb,
sizeof(struct udphdr),
@@ -1186,7 +1185,7 @@ try_again:
if (inet->cmsg_flags)
ip_cmsg_recv(msg, skb);
- err = copied;
+ err = len;
if (flags & MSG_TRUNC)
err = ulen;
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -322,7 +322,7 @@ int udpv6_recvmsg(struct kiocb *iocb, struct sock *sk,
struct ipv6_pinfo *np = inet6_sk(sk);
struct inet_sock *inet = inet_sk(sk);
struct sk_buff *skb;
- unsigned int ulen, copied;
+ unsigned int ulen;
int peeked;
int err;
int is_udplite = IS_UDPLITE(sk);
@@ -341,10 +341,9 @@ try_again:
goto out;
ulen = skb->len - sizeof(struct udphdr);
- copied = len;
- if (copied > ulen)
- copied = ulen;
- else if (copied < ulen)
+ if (len > ulen)
+ len = ulen;
+ else if (len < ulen)
msg->msg_flags |= MSG_TRUNC;
is_udp4 = (skb->protocol == htons(ETH_P_IP));
@@ -355,14 +354,14 @@ try_again:
* coverage checksum (UDP-Lite), do it before the copy.
*/
- if (copied < ulen || UDP_SKB_CB(skb)->partial_cov) {
+ if (len < ulen || UDP_SKB_CB(skb)->partial_cov) {
if (udp_lib_checksum_complete(skb))
goto csum_copy_err;
}
if (skb_csum_unnecessary(skb))
err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr),
- msg->msg_iov, copied );
+ msg->msg_iov,len);
else {
err = skb_copy_and_csum_datagram_iovec(skb, sizeof(struct udphdr), msg->msg_iov);
if (err == -EINVAL)
@@ -411,7 +410,7 @@ try_again:
datagram_recv_ctl(sk, msg, skb);
}
- err = copied;
+ err = len;
if (flags & MSG_TRUNC)
err = ulen;
^ permalink raw reply
* Re: [net-next-2.6, v4 1/3] ethtool: Introduce n-tuple filter programming support
From: Peter P Waskiewicz Jr @ 2010-02-11 7:07 UTC (permalink / raw)
To: Patrick McHardy
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <4B73A0BE.1060407@trash.net>
On Wed, 2010-02-10 at 22:16 -0800, Patrick McHardy wrote:
> Waskiewicz Jr, Peter P wrote:
> > On Wed, 10 Feb 2010, Patrick McHardy wrote:
> >
> > Thanks for the review Patrick. Comments inline.
> >
> > -PJ
> >
> >> Jeff Kirsher wrote:
> >>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> >>> index ef4a2d8..4e9ef85 100644
> >>> --- a/include/linux/ethtool.h
> >>> +++ b/include/linux/ethtool.h
> >>> +struct ethtool_rx_ntuple_list {
> >>> +#define ETHTOOL_MAX_NTUPLE_LIST_ENTRY 1024
> >>> +#define ETHTOOL_MAX_NTUPLE_STRING_PER_ENTRY 14
> >>> + struct list_head list;
> >>> + int count;
> >> unsigned int seems more appropriate.
> >
> > Really? It's a count of the number of cached filters. Is it just so we
> > don't overflow? I don't have any strong preference, so I can update this.
>
> Mainly because I don't think we can have a negative number
> of filters :)
Heh, good point.
> >>> u32 ethtool_op_get_flags(struct net_device *dev)
> >>> {
> >>> @@ -139,6 +139,11 @@ int ethtool_op_set_flags(struct net_device *dev, u32 data)
> >>> else
> >>> dev->features &= ~NETIF_F_LRO;
> >>>
> >>> + if (data & ETH_FLAG_NTUPLE)
> >>> + dev->features |= NETIF_F_NTUPLE;
> >>> + else
> >>> + dev->features &= ~NETIF_F_NTUPLE;
> >> Shouldn't this check for the real capabilities of the device first?
> >
> > The userspace side does before it calls the ioctl. It will abort with a
> > -EOPNOTSUPP (just tested with igb - properly aborted).
>
> I think this check belongs into the kernel. You already have these
> two checks in ethtool_set_rx_ntuple():
Ok, I see your point.
> > + if (!ops->set_rx_ntuple)
> > + return -EOPNOTSUPP;
> > +
> > + if (!(dev->features & NETIF_F_NTUPLE))
> > + return -EINVAL;
>
> Moving the check for ops->set_rx_ntuple to ethtool_op_set_flags()
> should be enough.
Dave wants me to fix up the caching, I'll look at including this in the
same patchset. It will also require checks in other drivers that
implement their own set_flags (like ixgbe) to take into account the
ethtool_op_set_flags() return code. ixgbe currently doesn't, which is
not correct.
> >>> +static int ethtool_get_rx_ntuple(struct net_device *dev, void __user *useraddr)
> >>> +{
> >>> + struct ethtool_gstrings gstrings;
> >>> + const struct ethtool_ops *ops = dev->ethtool_ops;
> >>> + struct ethtool_rx_ntuple_flow_spec_container *fsc;
> >>> + u8 *data;
> >>> + char *p;
> >>> + int ret, i, num_strings = 0;
> >>> +
> >>> + if (!ops->get_sset_count)
> >>> + return -EOPNOTSUPP;
> >>> +
> >>> + if (copy_from_user(&gstrings, useraddr, sizeof(gstrings)))
> >>> + return -EFAULT;
> >>> +
> >>> + ret = ops->get_sset_count(dev, gstrings.string_set);
> >>> + if (ret < 0)
> >>> + return ret;
> >>> +
> >>> + gstrings.len = ret;
> >>> +
> >>> + data = kmalloc(gstrings.len * ETH_GSTRING_LEN, GFP_USER);
> >>> + if (!data)
> >>> + return -ENOMEM;
> >>> +
> >>> + if (ops->get_rx_ntuple) {
> >>> + /* driver-specific filter grab */
> >>> + ret = ops->get_rx_ntuple(dev, gstrings.string_set, data);
> >>> + goto copy;
> >>> + }
> >>> +
> >>> + /* default ethtool filter grab */
> >>> + i = 0;
> >>> + p = (char *)data;
> >>> + list_for_each_entry(fsc, &dev->ethtool_ntuple_list.list, list) {
> >>> + sprintf(p, "Filter %d:\n", i);
> >> Providing a textual representation from within the kernel doesn't
> >> seem like a good interface to me. If userspace wants to do anything
> >> but simply display them, it will have to parse them again. Additionally
> >> it seems a driver providing a ->get_rx_ntuple() callback would have
> >> to duplicate the entire conversion code, which is error prone.
> >
> > The goal was to give a generic way to dump what was programmed, if an
> > underlying driver didn't want to implement the ->get_rx_ntuple()
> > operation. The two ways I could think of doing it was dump the list the
> > way I did, and provide a strings blob to ethtool (like stats), or try and
> > package the structs into a list, copy that to userspace, and let ethtool
> > generate the blobs.
> >
> > I agree that an underlying driver will have much of the same in terms of
> > what it generates, but it will not be restricted to how it stores the
> > items. In other words, if ixgbe wanted to retrieve all 8192 filters, we
> > could avoid the caching altogether, and pull directly from HW when the
> > call is made from ethtool. One way or another, there's going to be a big
> > amount of copied data from kernel space to user space. This was the
> > approach I thougt would be the most useful without defining a kernel to
> > userspace chain of flow spec structs.
>
> My main concern is that its hard for userspace to do anything with
> this data except print it. By using a binary representation the
> kernel code should get simpler and less prone to potential
> inconsistencies within drivers and make it more useful to userspace
> at the same time.
It would be easier to change ethtool in userspace to reformat data.
However, some drivers/hardware may represent data in a different way for
their filters, much like the ethtool stats (ethtool -S). I see this as
a hardware-specific thing, so letting the kernel provide the strings is
the most flexible, since it's the most representative of the hardware.
Cheers,
-PJ
^ permalink raw reply
* Re: [PATCH 0/3 v4] macvtap driver
From: Arnd Bergmann @ 2010-02-11 7:12 UTC (permalink / raw)
To: Ed Swierk; +Cc: Sridhar Samudrala, netdev
In-Reply-To: <9ae48b021002101642w752bbc93vbdcd71151dfb8cb7@mail.gmail.com>
On Thursday 11 February 2010 01:42:04 Ed Swierk wrote:
> On Wed, Feb 10, 2010 at 6:50 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > I think we also need to ensure the device doesn't go away, which
> > was one of the reasons for the rcu_read_lock_bh() earlier.
>
> This may be veering far off into the weeds, but I'm wondering if you
> considered making macvtap devices behave more like tap devices.
> Specifically, the application would open /dev/net/macvtap and send it
> an ioctl with the name of the macvtap interface, the name of the lower
> interface to attach to, the MAC address, etc; this would cause the
> macvtap interface to spring into existence. The macvtap interface
> would go away when the application exits or closes the file.
No, I never considered this. In fact, this behavior of tun/tap
is what makes that driver have really complex lifetime rules (more
so than macvtap) and causes all sorts of problems if you want to
manage unprivileged users accessing different outgoing interfaces.
> The tricky part here would be noticing when the lower interface goes
> away, and (ideally) reattaching when an interface with the same name
> reappears.
The first part is not so hard, the second part I'd rather not do.
> I think the advantage of this approach is that it better fits the way
> applications like qemu and libvirt use tap interfaces. Unlike the
> current approach, however, this wouldn't allow creating a macvtap
> interface and keep it around independently of the application using
> it. Is it desirable to support this use case?
I think it's very useful that you can set up static interfaces and give
them to a user (or group) that are then able to use these interfaces
without getting any network privileges beyond that.
Another reason for having one chardev per interface is to support
multiple open files for the same interface. I want to use that as
an easy way to support multi-queue NICs.
Arnd
^ permalink raw reply
* RE: [PATCH 0/3] Provide a zero-copy method on KVM virtio-net.
From: Xin, Xiaohui @ 2010-02-11 7:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mingo@elte.hu, mst@redhat.com,
jdike@c2.user-mode-linux.org
In-Reply-To: <201002101440.05063.arnd@arndb.de>
>>On Wednesday 10 February 2010, Xin Xiaohui wrote:
> >The idea is simple, just to pin the guest VM user space and then
> >let host NIC driver has the chance to directly DMA to it.
> >The patches are based on vhost-net backend driver. We add a device
> >which provides proto_ops as sendmsg/recvmsg to vhost-net to
> >send/recv directly to/from the NIC driver. KVM guest who use the
> >vhost-net backend may bind any ethX interface in the host side to
> >get copyless data transfer thru guest virtio-net frontend.
>>
>> We provide multiple submits and asynchronous notifiicaton to
> >vhost-net too.
>This does a lot of things that I had planned for macvtap. It's
>great to hear that you have made this much progress.
>
>However, I'd hope that we could combine this with the macvtap driver,
>which would give us zero-copy transfer capability both with and
>without vhost, as well as (tx at least) when using multiple guests
>on a macvlan setup.
You mean the zero-copy can work with macvtap driver without vhost.
May you give me some detailed info about your macvtap driver and the
relationship between vhost and macvtap to make me have a clear picture then?
>For transmit, it should be fairly straightforward to hook up
>your zero-copy method and the vhost-net interface into the
>macvtap driver.
>
>You have simplified the receiv path significantly by assuming
>that the entire netdev can receive into a single guest, right?
Yes.
>I'm assuming that the idea is to allow VMDq adapters to simply
>show up as separate adapters and have the driver handle this
>in a hardware specific way.
Does the VMDq driver do so now?
>My plan for this was to instead move support for VMDq into the
>macvlan driver so we can transparently use VMDq on hardware where
>available, including zero-copy receives, but fall back to software
>operation on non-VMDq hardware.
Arnd
^ permalink raw reply
* Re: [net-next-2.6, v4 1/3] ethtool: Introduce n-tuple filter programming support
From: Patrick McHardy @ 2010-02-11 7:53 UTC (permalink / raw)
To: Peter P Waskiewicz Jr
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <1265872027.4501.97.camel@localhost>
Peter P Waskiewicz Jr wrote:
> On Wed, 2010-02-10 at 22:16 -0800, Patrick McHardy wrote:
>>> I agree that an underlying driver will have much of the same in terms of
>>> what it generates, but it will not be restricted to how it stores the
>>> items. In other words, if ixgbe wanted to retrieve all 8192 filters, we
>>> could avoid the caching altogether, and pull directly from HW when the
>>> call is made from ethtool. One way or another, there's going to be a big
>>> amount of copied data from kernel space to user space. This was the
>>> approach I thougt would be the most useful without defining a kernel to
>>> userspace chain of flow spec structs.
>> My main concern is that its hard for userspace to do anything with
>> this data except print it. By using a binary representation the
>> kernel code should get simpler and less prone to potential
>> inconsistencies within drivers and make it more useful to userspace
>> at the same time.
>
> It would be easier to change ethtool in userspace to reformat data.
I'm thinking more about other potential users of this interface.
> However, some drivers/hardware may represent data in a different way for
> their filters, much like the ethtool stats (ethtool -S). I see this as
> a hardware-specific thing, so letting the kernel provide the strings is
> the most flexible, since it's the most representative of the hardware.
I think there's a difference between stats and filters. In case of
filters, userspace already needs to know the representation without
knowing the specific driver in order to send them to the kernel, so
it needs to use a common representation unless I'm missing something.
But fair enough, I just wanted to state my concerns, which might of
course be wrong.
^ permalink raw reply
* Re: [PATCH 3/3] via-velocity: Fix races on shared interrupts
From: Simon Kagstrom @ 2010-02-11 8:05 UTC (permalink / raw)
To: Laurent Chavey; +Cc: netdev, davem, davej, ben
In-Reply-To: <97949e3e1002100941s34010bdfu1a52f95693d3a9e9@mail.gmail.com>
Hi Laurent!
On Wed, 10 Feb 2010 09:41:59 -0800
Laurent Chavey <chavey@google.com> wrote:
> > - spin_lock(&vptr->lock);
> > + /* Check if the lock is taken, and if so ignore the interrupt. This
> > + * can happen with shared interrupts, where the other device can
> > + * interrupt during velocity_poll (where the lock is held).
> > + *
> > + * With spinlock debugging active on a uniprocessor, this will give
> > + * a warning which can safely be ignored.
> > + */
> > + if (!spin_trylock(&vptr->lock))
> > + return IRQ_NONE;
>
> does the thread handling the interrupts check that an new
> interrupts was received while it was servicing a previous one ?
> wondering if there is a potential for an event that generates the interrupt
> to be missed.
I should say that this particular part of the patch was reworked in
version 2, which David took in here:
http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commitdiff;h=3f2e8d9f13246382fbda6f03178eef867a9bfbe2
Anyway, velocity_poll will try to empty all events within it's budget
from the device and is executing with the device interrupt turned off
(and now also with the local processor interrupts off). If something
would be posted when it's exiting, that's fine since it either
1) Consumed it's entire budget, in which case it will stay in the
polling mode anyway
or,
2) Didn't consume the budget and will then turn on the interrupt
again and get the new event promptly.
You are right about the code above though, that one is racy as David
explained here:
http://permalink.gmane.org/gmane.linux.network/151578
// Simon
^ permalink raw reply
* [patch] 9p: fix return value of rdma_request()
From: Dan Carpenter @ 2010-02-11 8:20 UTC (permalink / raw)
To: netdev
Cc: Eric Van Hensbergen, Abhishek Kulkarni, Venkateswararao Jujjuri,
Latchesar Ionkov, David S. Miller, kernel-janitors
rdma_request() is supposed to return negative values on error. This change
means that if a kmalloc() fails it returns -ENOMEM. The original code returned
certain negative values, zero on success and zero on kmalloc() failures.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
I was just reading the code and I noticed this bug. I haven't tested it because I
don't have the hardware.
diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
index 2c95a89..589e340 100644
--- a/net/9p/trans_rdma.c
+++ b/net/9p/trans_rdma.c
@@ -419,7 +419,7 @@ static int rdma_request(struct p9_client *client, struct p9_req_t *req)
struct p9_trans_rdma *rdma = client->trans;
struct ib_send_wr wr, *bad_wr;
struct ib_sge sge;
- int err = 0;
+ int err = -ENOMEM;
unsigned long flags;
struct p9_rdma_context *c = NULL;
struct p9_rdma_context *rpl_context = NULL;
^ permalink raw reply related
* RE: [PATCH 0/3] Provide a zero-copy method on KVM virtio-net.
From: Xin, Xiaohui @ 2010-02-11 8:54 UTC (permalink / raw)
To: Xin, Xiaohui, netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1265802540-6122-1-git-send-email-xiaohui.xin@intel.com>
Will be in a vacation during 2/13~2/20, so email may be very slow or no replied
for your comments. But please don't hesitate to comment more, and I will address
them after the vacation. :-)
Thanks
Xiaohui
-----Original Message-----
From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of Xin Xiaohui
Sent: Wednesday, February 10, 2010 7:49 PM
To: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org; mingo@elte.hu; mst@redhat.com; jdike@c2.user-mode-linux.org
Subject: [PATCH 0/3] Provide a zero-copy method on KVM virtio-net.
The idea is simple, just to pin the guest VM user space and then
let host NIC driver has the chance to directly DMA to it.
The patches are based on vhost-net backend driver. We add a device
which provides proto_ops as sendmsg/recvmsg to vhost-net to
send/recv directly to/from the NIC driver. KVM guest who use the
vhost-net backend may bind any ethX interface in the host side to
get copyless data transfer thru guest virtio-net frontend.
We provide multiple submits and asynchronous notifiicaton to
vhost-net too.
Our goal is to improve the bandwidth and reduce the CPU usage.
Exact performance data will be provided later. But for simple
test with netperf, we found bindwidth up and CPU % up too,
but the bindwidth up ratio is much more than CPU % up ratio.
What we have not done yet:
To support GRO
Performance tuning
--
To unsubscribe from this list: send the line "unsubscribe kvm" 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
* [net-next-2.6 PATCH v5 0/3] Introduce n-tuple ethtool support
From: robert @ 2010-02-11 8:02 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, netdev, gospo
In-Reply-To: <20100211020310.23436.85885.stgit@localhost.localdomain>
Nice. Next step would be to control the RSS-traffic. Doing this we
can easily get total separation between "control" and "data" plane.
We did a dirty hack to separate out bgp/ssh to one queue/cpu.
I think just a RSS_mask would be a good start - each bit controlling
if queue should receive RSS traffic or not.
Cheers
--ro
Jeff Kirsher writes:
> One more round of fixes, based on feedback from Patrick McHardy
>
> 1) Change the list count to an unsigned value
> 2) Fix a memory leak
> 3) Removed an unnecessary list traversal in the ethtool core
> 4) Moved all list destruction to a helper function, allowing the driver
> to control when it clears the list (aside from when free_netdev() kills
> the cached list).
>
> ---
>
> Peter Waskiewicz (3):
> ixgbe: Bump driver version up
> ixgbe: Add support for the new ethtool n-tuple programming interface
> ethtool: Introduce n-tuple filter programming support
>
>
> drivers/net/ixgbe/ixgbe.h | 4
> drivers/net/ixgbe/ixgbe_82599.c | 106 ++++++++++--
> drivers/net/ixgbe/ixgbe_ethtool.c | 111 ++++++++++++
> drivers/net/ixgbe/ixgbe_main.c | 18 ++
> drivers/net/ixgbe/ixgbe_type.h | 9 +
> include/linux/ethtool.h | 50 ++++++
> include/linux/netdevice.h | 3
> net/core/dev.c | 6 +
> net/core/ethtool.c | 332 +++++++++++++++++++++++++++++++++++++
> 9 files changed, 619 insertions(+), 20 deletions(-)
>
> --
> Cheers,
> Jeff
> --
> 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]xfrm: Flushing empty SAD generates false events
From: jamal @ 2010-02-11 9:51 UTC (permalink / raw)
To: David Miller, Herbert Xu; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 422 bytes --]
To see the effect make sure you have an empty SAD.
-On window1 "ip xfrm mon"
-on window2 issue "ip xfrm state flush"
You get prompt back in window1
and you see the flush event on window2.
With this fix, you still get prompt on window1 but no
event on window2.
I was tempted to return -ESRCH on window1 (which would
show "RTNETLINK answers: No such process") but didnt want
to change current behavior.
cheers,
jamal
[-- Attachment #2: SAD-flush-ev --]
[-- Type: text/plain, Size: 2313 bytes --]
commit 5f3dd4a772326166e1bcf54acc2391df00dc7ab5
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Thu Feb 11 04:41:36 2010 -0500
xfrm: Flushing empty SAD generates false events
To see the effect make sure you have an empty SAD.
On window1 "ip xfrm mon" and on window2 issue "ip xfrm state flush"
You get prompt back in window1 and you see the flush event on window2.
With this fix, you still get prompt on window1 but no event on window2.
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 76fa6fe..0f53e60 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1751,7 +1751,7 @@ static int pfkey_flush(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hd
audit_info.secid = 0;
err = xfrm_state_flush(net, proto, &audit_info);
if (err)
- return err;
+ return 0;
c.data.proto = proto;
c.seq = hdr->sadb_msg_seq;
c.pid = hdr->sadb_msg_pid;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index b36cc34..f50ee9b 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -603,13 +603,14 @@ xfrm_state_flush_secctx_check(struct net *net, u8 proto, struct xfrm_audit *audi
int xfrm_state_flush(struct net *net, u8 proto, struct xfrm_audit *audit_info)
{
- int i, err = 0;
+ int i, err = 0, cnt = 0;
spin_lock_bh(&xfrm_state_lock);
err = xfrm_state_flush_secctx_check(net, proto, audit_info);
if (err)
goto out;
+ err = -ESRCH;
for (i = 0; i <= net->xfrm.state_hmask; i++) {
struct hlist_node *entry;
struct xfrm_state *x;
@@ -626,13 +627,16 @@ restart:
audit_info->sessionid,
audit_info->secid);
xfrm_state_put(x);
+ if (!err)
+ cnt++;
spin_lock_bh(&xfrm_state_lock);
goto restart;
}
}
}
- err = 0;
+ if (cnt)
+ err = 0;
out:
spin_unlock_bh(&xfrm_state_lock);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 943c871..7c87004 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1525,7 +1525,7 @@ static int xfrm_flush_sa(struct sk_buff *skb, struct nlmsghdr *nlh,
audit_info.secid = NETLINK_CB(skb).sid;
err = xfrm_state_flush(net, p->proto, &audit_info);
if (err)
- return err;
+ return 0;
c.data.proto = p->proto;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
^ permalink raw reply related
* [PATCH]IPv6: Delete redundant counter of IPSTATS_MIB_REASMFAILS
From: Shan Wei @ 2010-02-11 10:12 UTC (permalink / raw)
To: David Miller, kuznet, pekkas, jmorris, yoshfuji, Patrick McHardy,
eric.dumazet, ado
Cc: netdev@vger.kernel.org
When no more memory can be allocated, fq_find() will return NULL and
increase the value of IPSTATS_MIB_REASMFAILS. In this case,
ipv6_frag_rcv() also increase the value of IPSTATS_MIB_REASMFAILS.
So, the patch deletes redundant counter of IPSTATS_MIB_REASMFAILS in fq_find().
and deletes the unused parameter of idev.
Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
net/ipv6/reassembly.c | 13 ++++---------
1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 2cddea3..01a996f 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -237,8 +237,7 @@ out:
}
static __inline__ struct frag_queue *
-fq_find(struct net *net, __be32 id, struct in6_addr *src, struct in6_addr *dst,
- struct inet6_dev *idev)
+fq_find(struct net *net, __be32 id, struct in6_addr *src, struct in6_addr *dst)
{
struct inet_frag_queue *q;
struct ip6_create_arg arg;
@@ -254,13 +253,9 @@ fq_find(struct net *net, __be32 id, struct in6_addr *src, struct in6_addr *dst,
q = inet_frag_find(&net->ipv6.frags, &ip6_frags, &arg, hash);
if (q == NULL)
- goto oom;
+ return NULL;
return container_of(q, struct frag_queue, q);
-
-oom:
- IP6_INC_STATS_BH(net, idev, IPSTATS_MIB_REASMFAILS);
- return NULL;
}
static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
@@ -606,8 +601,8 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
if (atomic_read(&net->ipv6.frags.mem) > net->ipv6.frags.high_thresh)
ip6_evictor(net, ip6_dst_idev(skb_dst(skb)));
- if ((fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr,
- ip6_dst_idev(skb_dst(skb)))) != NULL) {
+ fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr);
+ if (fq != NULL) {
int ret;
spin_lock(&fq->q.lock);
--
1.6.3.3
^ permalink raw reply related
* [PATCH]xfrm: Flushing empty SPD generates false events
From: jamal @ 2010-02-11 10:53 UTC (permalink / raw)
To: David Miller, Herbert Xu; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 99 bytes --]
Observed similar behavior on SPD as previouly seen on SAD flushing..
This fixes it.
cheers,
jamal
[-- Attachment #2: SPD-flush-ev --]
[-- Type: text/plain, Size: 2904 bytes --]
commit 428b20432dc31bc2e01a94cd451cf5a2c00d2bf4
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Thu Feb 11 05:49:38 2010 -0500
xfrm: Flushing empty SPD generates false events
To see the effect make sure you have an empty SPD.
On window1 "ip xfrm mon" and on window2 issue "ip xfrm policy flush"
You get prompt back in window1 and you see the flush event on window2.
With this fix, you still get prompt on window1 but no event on window2.
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 0f53e60..978f78b 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2713,7 +2713,7 @@ static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, struct sadb_msg
audit_info.secid = 0;
err = xfrm_policy_flush(net, XFRM_POLICY_TYPE_MAIN, &audit_info);
if (err)
- return err;
+ return 0;
c.data.type = XFRM_POLICY_TYPE_MAIN;
c.event = XFRM_MSG_FLUSHPOLICY;
c.pid = hdr->sadb_msg_pid;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 0ecb16a..eb870fc 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -771,7 +771,8 @@ xfrm_policy_flush_secctx_check(struct net *net, u8 type, struct xfrm_audit *audi
int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
{
- int dir, err = 0;
+ int dir, err = 0, cnt = 0;
+ struct xfrm_policy *dp;
write_lock_bh(&xfrm_policy_lock);
@@ -789,8 +790,10 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
&net->xfrm.policy_inexact[dir], bydst) {
if (pol->type != type)
continue;
- __xfrm_policy_unlink(pol, dir);
+ dp = __xfrm_policy_unlink(pol, dir);
write_unlock_bh(&xfrm_policy_lock);
+ if (dp)
+ cnt++;
xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,
audit_info->sessionid,
@@ -809,8 +812,10 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
bydst) {
if (pol->type != type)
continue;
- __xfrm_policy_unlink(pol, dir);
+ dp = __xfrm_policy_unlink(pol, dir);
write_unlock_bh(&xfrm_policy_lock);
+ if (dp)
+ cnt++;
xfrm_audit_policy_delete(pol, 1,
audit_info->loginuid,
@@ -824,6 +829,8 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
}
}
+ if (!cnt)
+ err = -ESRCH;
atomic_inc(&flow_cache_genid);
out:
write_unlock_bh(&xfrm_policy_lock);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 7c87004..b0fb7d3 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1677,7 +1677,7 @@ static int xfrm_flush_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
audit_info.secid = NETLINK_CB(skb).sid;
err = xfrm_policy_flush(net, type, &audit_info);
if (err)
- return err;
+ return 0;
c.data.type = type;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
^ permalink raw reply related
* Re: [PATCH 2/4] C/R: Basic support for network namespaces and devices (v3)
From: Louis Rilling @ 2010-02-11 11:02 UTC (permalink / raw)
To: Dan Smith
Cc: containers-qjLDD68F18O7TbgM5vRIOg, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87ljf1gemh.fsf-FLMGYpZoEPULwtHQx/6qkW3U47Q5hpJU@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3772 bytes --]
Hi Dan,
On 10/02/10 9:55 -0800, Dan Smith wrote:
> Guilt dropped the new checkpoint_dev.c file when I switched to the
> newer branch. Sorry about that. Updated patch included below.
>
[...]
> diff --git a/net/checkpoint_dev.c b/net/checkpoint_dev.c
> new file mode 100644
> index 0000000..0dddd15
> --- /dev/null
> +++ b/net/checkpoint_dev.c
[...]
> +
> +static struct nlmsghdr *rtnl_get_response(struct socket *rtnl,
> + struct sk_buff **skb)
> +{
> + int ret;
> + long timeo = MAX_SCHEDULE_TIMEOUT;
> + struct nlmsghdr *nlh;
> +
> + ret = sk_wait_data(rtnl->sk, &timeo);
> + if (!ret)
> + return ERR_PTR(-EPIPE);
> +
> + *skb = skb_dequeue(&rtnl->sk->sk_receive_queue);
> + if (!*skb)
> + return ERR_PTR(-EPIPE);
> +
> + ret = -EINVAL;
> + nlh = nlmsg_hdr(*skb);
> + if (!nlh)
> + goto err;
> +
> + if (nlh->nlmsg_type == NLMSG_ERROR) {
> + struct nlmsgerr *errmsg = nlmsg_data(nlh);
> + ret = errmsg->error;
> + goto err;
> + }
> +
> + return nlh;
> + err:
> + kfree_skb(*skb);
> + *skb = NULL;
> +
> + return ERR_PTR(ret);
> +}
> +
[...]
> +
> +static struct sk_buff *new_link_message(char *this_name, char *peer_name)
> +{
> + int ret = -ENOMEM;
> + int flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_ACK;
> + struct nlmsghdr *nlh;
> + struct sk_buff *skb;
> + struct ifinfomsg *ifm;
> + struct nlattr *linkinfo;
> +
> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!skb)
> + goto out;
> +
> + nlh = nlmsg_put(skb, 0, 0, RTM_NEWLINK, sizeof(*ifm), flags);
> + if (!nlh)
> + goto out;
> +
> + ifm = nlmsg_data(nlh);
> + memset(ifm, 0, sizeof(*ifm));
> +
> + ret = nla_put_string(skb, IFLA_IFNAME, this_name);
> + if (ret)
> + goto out;
> +
> + ret = -ENOMEM;
> +
> + linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
> + if (!linkinfo)
> + goto out;
> +
> + if (nla_put_string(skb, IFLA_INFO_KIND, "veth") < 0)
> + goto out;
> +
> + ret = veth_peer_data(skb, peer_name);
> + if (ret < 0)
> + goto out;
> +
> + nla_nest_end(skb, linkinfo);
> + nlmsg_end(skb, nlh);
> +
> + out:
> + if (ret < 0) {
> + kfree(skb);
I'm definitely not a network expert, but this kfree(skb) should probably be
replaced by kfree_skb(skb).
> + skb = ERR_PTR(ret);
> + }
> +
> + return skb;
> +}
> +
> +static struct net_device *new_veth_pair(char *this_name, char *peer_name)
> +{
> + int ret = -ENOMEM;
> + struct socket *rtnl;
> + struct sk_buff *skb = NULL;
> + struct nlmsghdr *nlh;
> + struct msghdr msg;
> + struct kvec kvec;
> +
> + skb = new_link_message(this_name, peer_name);
> + if (IS_ERR(skb)) {
> + ret = PTR_ERR(skb);
> + ckpt_debug("failed to create new link message: %i\n", ret);
> + skb = NULL;
> + goto out;
> + }
> +
> + memset(&msg, 0, sizeof(msg));
> + kvec.iov_len = skb->len;
> + kvec.iov_base = skb->head;
> +
> + rtnl = rtnl_open();
> + if (IS_ERR(rtnl)) {
> + ret = PTR_ERR(rtnl);
> + ckpt_debug("Unable to open rtnetlink socket: %i\n", ret);
> + goto out_noclose;
> + }
> +
> + ret = kernel_sendmsg(rtnl, &msg, &kvec, 1, kvec.iov_len);
> + if (ret < 0)
> + goto out;
> + else if (ret != skb->len) {
> + ret = -EIO;
> + goto out;
> + }
> +
> + /* Free the send skb to make room for the receive skb */
> + kfree(skb);
Ditto.
> +
> + nlh = rtnl_get_response(rtnl, &skb);
> + if (IS_ERR(nlh)) {
> + ret = PTR_ERR(nlh);
> + ckpt_debug("RTNETLINK said: %i\n", ret);
> + }
> + out:
> + rtnl_close(rtnl);
> + out_noclose:
> + kfree(skb);
Ditto.
Thanks,
Louis
[...]
--
Dr Louis Rilling Kerlabs
Skype: louis.rilling Batiment Germanium
Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes
http://www.kerlabs.com/ 35700 Rennes
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 206 bytes --]
_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers
^ permalink raw reply
* [PATCH][KS8695] Mark network interface as running on ifconfig up
From: Yegor Yefremov @ 2010-02-11 11:12 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: netdev, Andrew Victor
[KS8695] Mark network interface as running on ifconfig up
Without netif_carrier_on() network interface will not transmit any packets
after ifconfig down and subsequent ifconfig up.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Index: linux-2.6.33-rc7/drivers/net/arm/ks8695net.c
===================================================================
--- linux-2.6.33-rc7.orig/drivers/net/arm/ks8695net.c
+++ linux-2.6.33-rc7/drivers/net/arm/ks8695net.c
@@ -1371,6 +1371,7 @@ ks8695_open(struct net_device *ndev)
napi_enable(&ksp->napi);
netif_start_queue(ndev);
+ netif_carrier_on(ndev);
return 0;
}
^ permalink raw reply
* Re: [PATCH] l2tp: Fix a UDP socket reference count bug in the pppol2tp driver
From: James Chapman @ 2010-02-11 11:32 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100128.060717.99647383.davem@davemloft.net>
David Miller wrote:
> From: James Chapman <jchapman@katalix.com>
> Date: Wed, 27 Jan 2010 13:14:56 +0000
>
>> Does the above help?
>
> Thanks for the detailed explanation, I'll take another look
> at this.
Did you get a chance to look at this?
fyi - I'm getting ready to submit a patch series that adds L2TPv3
support. I've been sitting on these for too long and have finally found
time to prep them for review. Previous version was posted as RFC a year
ago, archived here: http://marc.info/?l=linux-netdev&m=123532490429538&w=2
Should I wait for my previous fix to be reviewed before posting the new
code? Or I could bundle that patch in the series - whichever works for you.
Thanks
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* [net-next PATCH v3 0/3] net: TCP thin-stream latency-improving modifications
From: Andreas Petlund @ 2010-02-11 12:07 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: Ilpo Järvinen, Eric Dumazet, Arnd Hannemann, LKML,
shemminger, David Miller, william.allen.simpson, damian
This is a series of patches enabling non-intrusive, dynamically triggered modifications that improve retransmission latencies for thin streams.
The patches have been modified according to the feedback from the last round of discussions.
Major changes:
-Possible to disable mechanisms by socket option
-Socket option value boundary check
Cheers,
Andreas Petlund
^ permalink raw reply
* [net-next PATCH v3 1/3] net: TCP thin-stream detection
From: Andreas Petlund @ 2010-02-11 12:07 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: Ilpo Järvinen, Eric Dumazet, Arnd Hannemann, LKML,
shemminger, David Miller, william.allen.simpson, damian
No changes here from v2.
Signed-off-by: Andreas Petlund <apetlund@simula.no>
---
include/net/tcp.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 87d164b..e5e2056 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1386,6 +1386,13 @@ static inline void tcp_highest_sack_combine(struct sock *sk,
tcp_sk(sk)->highest_sack = new;
}
+/* Determines whether this is a thin stream (which may suffer from
+ * increased latency). Used to trigger latency-reducing mechanisms.*/
+static inline unsigned int tcp_stream_is_thin(struct tcp_sock *tp)
+{
+ return tp->packets_out < 4 && !tcp_in_initial_slowstart(tp);
+}
+
/* /proc */
enum tcp_seq_states {
TCP_SEQ_STATE_LISTENING,
--
1.6.3.3
^ permalink raw reply related
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