* Re: [net PATCH v2 1/2] net: netprio: fix files lock and remove useless d_path bits
From: Neil Horman @ 2012-08-14 12:56 UTC (permalink / raw)
To: John Fastabend; +Cc: viro, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814024320.6983.92222.stgit@jf-dev1-dcblab>
On Mon, Aug 13, 2012 at 07:43:21PM -0700, John Fastabend wrote:
> Add lock to prevent a race with a file closing and also remove
> useless and ugly sscanf code. The extra code was never needed
> and the case it supposedly protected against is in fact handled
> correctly by sock_from_file as pointed out by Al Viro.
>
> CC: Neil Horman <nhorman@tuxdriver.com>
> Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>
> net/core/netprio_cgroup.c | 22 ++++------------------
> 1 files changed, 4 insertions(+), 18 deletions(-)
>
> diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
> index ed0c043..f65dba3 100644
> --- a/net/core/netprio_cgroup.c
> +++ b/net/core/netprio_cgroup.c
> @@ -277,12 +277,6 @@ out_free_devname:
> void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
> {
> struct task_struct *p;
> - char *tmp = kzalloc(sizeof(char) * PATH_MAX, GFP_KERNEL);
> -
> - if (!tmp) {
> - pr_warn("Unable to attach cgrp due to alloc failure!\n");
> - return;
> - }
>
> cgroup_taskset_for_each(p, cgrp, tset) {
> unsigned int fd;
> @@ -296,32 +290,24 @@ void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
> continue;
> }
>
> - rcu_read_lock();
> + spin_lock(&files->file_lock);
> fdt = files_fdtable(files);
> for (fd = 0; fd < fdt->max_fds; fd++) {
> - char *path;
> struct file *file;
> struct socket *sock;
> - unsigned long s;
> - int rv, err = 0;
> + int err;
>
> file = fcheck_files(files, fd);
> if (!file)
> continue;
>
> - path = d_path(&file->f_path, tmp, PAGE_SIZE);
> - rv = sscanf(path, "socket:[%lu]", &s);
> - if (rv <= 0)
> - continue;
> -
> sock = sock_from_file(file, &err);
> - if (!err)
> + if (sock)
> sock_update_netprioidx(sock->sk, p);
> }
> - rcu_read_unlock();
> + spin_unlock(&files->file_lock);
> task_unlock(p);
> }
> - kfree(tmp);
> }
>
> static struct cftype ss_files[] = {
>
This looks ok to me, but I've already shown my inability to review code that
interfaces with VFS. Al, what do you think?
Neil
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [net PATCH v2 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: Neil Horman @ 2012-08-14 12:55 UTC (permalink / raw)
To: John Fastabend; +Cc: viro, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814024326.6983.97403.stgit@jf-dev1-dcblab>
On Mon, Aug 13, 2012 at 07:43:27PM -0700, John Fastabend wrote:
> A socket fd passed in a SCM_RIGHTS datagram was not getting
> updated with the new tasks cgrp prioidx. This leaves IO on
> the socket tagged with the old tasks priority.
>
> To fix this add a check in the scm recvmsg path to update the
> sock cgrp prioidx with the new tasks value.
>
> Thanks to Al Viro for catching this.
>
> CC: Neil Horman <nhorman@tuxdriver.com>
> Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>
> net/core/scm.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/net/core/scm.c b/net/core/scm.c
> index 8f6ccfd..a14d9e2 100644
> --- a/net/core/scm.c
> +++ b/net/core/scm.c
> @@ -249,6 +249,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
> struct file **fp = scm->fp->fp;
> int __user *cmfptr;
> int err = 0, i;
> + __u32 prioidx = task_netprioidx(current);
>
> if (MSG_CMSG_COMPAT & msg->msg_flags) {
> scm_detach_fds_compat(msg, scm);
> @@ -265,6 +266,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
> for (i=0, cmfptr=(__force int __user *)CMSG_DATA(cm); i<fdmax;
> i++, cmfptr++)
> {
> + struct socket *sock;
> int new_fd;
> err = security_file_receive(fp[i]);
> if (err)
> @@ -281,6 +283,9 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
> }
> /* Bump the usage count and install the file. */
> get_file(fp[i]);
> + sock = sock_from_file(fp[i], &err);
> + if (sock)
> + sock->sk->sk_cgrp_prioidx = prioidx;
nit: You can replace the prioidx variable above and this set with a call to
sock_update_netprioidx
Neil
^ permalink raw reply
* RE: [PATCH 3/5] net/mlx4_en: Call netif_carrier_off() after register_netdev()
From: Sathya.Perla @ 2012-08-14 12:36 UTC (permalink / raw)
To: shchepetkov, yevgenyp
Cc: davem, amirv, ogerlitz, alexg, netdev, linux-kernel, ldv-project
In-Reply-To: <1344940135-17079-4-git-send-email-shchepetkov@ispras.ru>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of Ilya Shchepetkov
>
>For carrier detection to work properly when binding the driver with a
>cable unplugged, netif_carrier_off() should be called after
>register_netdev(), not before.
>
>Calling netif_carrier_off() before register_netdev() was causing the
>network interface to miss a linkwatch pending event leading to an
>inconsistent state if the link is not up when interface is initialized.
ndo_open() may be called as soon register_netdev() completes...
When netif_carrier_off() is called *after* register_netdev(), isn't there
a possibility of a ndo_open()->netif_carrier_on() call racing this call, causing
incorrect results?
^ permalink raw reply
* Re: [patch net-next 01/16] net: introduce upper device lists
From: Jiri Pirko @ 2012-08-14 12:24 UTC (permalink / raw)
To: Flavio Leitner
Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
bhutchings, therbert, xiyou.wangcong, joe, gregory.v.rose,
john.r.fastabend, linux-rdma, linux-kernel, linux-s390, bridge
In-Reply-To: <20120813145217.38748c4f@obelix.rh>
Mon, Aug 13, 2012 at 07:52:17PM CEST, fbl@redhat.com wrote:
>On Mon, 13 Aug 2012 17:27:00 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> This lists are supposed to serve for storing pointers to all upper devices.
>> Eventually it will replace dev->master pointer which is used for
>> bonding, bridge, team but it cannot be used for vlan, macvlan where
>> there might be multiple "masters" present.
>>
>> New upper device list resolves this limitation. Also, the information
>> stored in lists is used for preventing looping setups like
>> "bond->somethingelse->samebond"
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>> include/linux/netdevice.h | 14 +++
>> net/core/dev.c | 232 ++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 244 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index a9db4f3..e7a07f8 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1173,6 +1173,8 @@ struct net_device {
>> * which this device is member of.
>> */
>>
>> + struct list_head upper_dev_list; /* List of upper devices */
>> +
>> /* Interface address info used in eth_type_trans() */
>> unsigned char *dev_addr; /* hw address, (before bcast
>> because most packets are
>> @@ -2611,6 +2613,18 @@ extern int netdev_max_backlog;
>> extern int netdev_tstamp_prequeue;
>> extern int weight_p;
>> extern int bpf_jit_enable;
>> +
>> +extern bool netdev_has_upper_dev(struct net_device *dev,
>> + struct net_device *upper_dev);
>> +extern bool netdev_has_any_upper_dev(struct net_device *dev);
>> +extern struct net_device *netdev_unique_upper_dev_get(struct net_device *dev);
>> +extern struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev);
>> +extern int netdev_upper_dev_link(struct net_device *dev,
>> + struct net_device *upper_dev);
>> +extern int netdev_unique_upper_dev_link(struct net_device *dev,
>> + struct net_device *upper_dev);
>> +extern void netdev_upper_dev_unlink(struct net_device *dev,
>> + struct net_device *upper_dev);
>> extern int netdev_set_master(struct net_device *dev, struct net_device *master);
>> extern int netdev_set_bond_master(struct net_device *dev,
>> struct net_device *master);
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 1f06df8..68db1ac 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4425,6 +4425,229 @@ static int __init dev_proc_init(void)
>> #endif /* CONFIG_PROC_FS */
>>
>>
>> +struct netdev_upper {
>> + struct net_device *dev;
>> + bool unique;
>
>unique is quite confusing here. I see that it is possible to have
>one unique and many non-unique linked in the list, so maybe rename
>to 'main_dev', 'master' or 'principal'...
>
>
>> + struct list_head list;
>> + struct rcu_head rcu;
>> +};
>> +
>> +static bool __netdev_has_upper_dev(struct net_device *dev,
>> + struct net_device *upper_dev,
>> + bool deep)
>> +{
>> + struct netdev_upper *upper;
>> +
>> + list_for_each_entry(upper, &dev->upper_dev_list, list) {
>> + if (upper->dev == upper_dev)
>> + return true;
>> + if (deep && __netdev_has_upper_dev(upper->dev, upper_dev, deep))
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +static struct netdev_upper *__netdev_find_upper(struct net_device *dev,
>> + struct net_device *upper_dev)
>> +{
>> + struct netdev_upper *upper;
>> +
>> + list_for_each_entry(upper, &dev->upper_dev_list, list) {
>> + if (upper->dev == upper_dev)
>> + return upper;
>> + }
>> + return NULL;
>> +}
>> +
>> +/**
>> + * netdev_has_upper_dev - Check if device is linked to an upper device
>> + * @dev: device
>> + * @upper_dev: upper device to check
>> + *
>> + * Find out if a device is linked to specified upper device and return true
>> + * in case it is. The caller must hold the RTNL semaphore.
>> + */
>> +bool netdev_has_upper_dev(struct net_device *dev,
>> + struct net_device *upper_dev)
>> +{
>> + ASSERT_RTNL();
>> +
>> + return __netdev_has_upper_dev(dev, upper_dev, false);
>> +}
>> +EXPORT_SYMBOL(netdev_has_upper_dev);
>> +
>> +/**
>> + * netdev_has_any_upper_dev - Check if device is linked to some device
>> + * @dev: device
>> + *
>> + * Find out if a device is linked to an upper device and return true in case
>> + * it is. The caller must hold the RTNL semaphore.
>> + */
>> +bool netdev_has_any_upper_dev(struct net_device *dev)
>> +{
>> + ASSERT_RTNL();
>> +
>> + return !list_empty(&dev->upper_dev_list);
>> +}
>> +EXPORT_SYMBOL(netdev_has_any_upper_dev);
>> +
>> +/**
>> + * netdev_unique_upper_dev_get - Get unique upper device
>> + * @dev: device
>> + *
>> + * Find a unique upper device and return pointer to it or NULL in case
>> + * it's not there. The caller must hold the RTNL semaphore.
>> + */
>> +struct net_device *netdev_unique_upper_dev_get(struct net_device *dev)
>> +{
>> + struct netdev_upper *upper;
>> +
>> + ASSERT_RTNL();
>> +
>> + if (list_empty(&dev->upper_dev_list))
>> + return NULL;
>> +
>> + upper = list_first_entry(&dev->upper_dev_list,
>> + struct netdev_upper, list);
>> + if (likely(upper->unique))
>> + return upper->dev;
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL(netdev_unique_upper_dev_get);
>> +
>> +/**
>> + * netdev_unique_upper_dev_get_rcu - Get unique upper device
>> + * @dev: device
>> + *
>> + * Find a unique upper device and return pointer to it or NULL in case
>> + * it's not there. The caller must hold the RCU read lock.
>> + */
>> +struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev)
>> +{
>> + struct netdev_upper *upper;
>> +
>> + upper = list_first_or_null_rcu(&dev->upper_dev_list,
>> + struct netdev_upper, list);
>> + if (likely(upper->unique))
>
>It will oopses here if 'upper' is NULL (i.e. no upper devices).
Fixed
>
>
>> + return upper->dev;
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL(netdev_unique_upper_dev_get_rcu);
>> +
>> +static int __netdev_upper_dev_link(struct net_device *dev,
>> + struct net_device *upper_dev, bool unique)
>> +{
>> + struct netdev_upper *upper;
>> +
>> + ASSERT_RTNL();
>> +
>> + if (dev == upper_dev)
>> + return -EBUSY;
>> + /*
>> + * To prevent loops, check if dev is not upper device to upper_dev.
>> + */
>> + if (__netdev_has_upper_dev(upper_dev, dev, true))
>> + return -EBUSY;
>> +
>> + if (__netdev_find_upper(dev, upper_dev))
>> + return -EEXIST;
>
>__netdev_has_upper_dev() can go all the way up finding the device and
>the __netdev_find_upper() just check the first level.
I do not think this ordering is somewhat inportant.
>
>I think it would be better to use:
>__netdev_find_upper_dev(,,deep=true/false)
>__netdev_has_upper(,)
>
>thanks,
>fbl
>
>> + if (unique && netdev_unique_upper_dev_get(dev))
>> + return -EBUSY;
>> +
>> + upper = kmalloc(sizeof(*upper), GFP_KERNEL);
>> + if (!upper)
>> + return -ENOMEM;
>> +
>> + upper->dev = upper_dev;
>> + upper->unique = unique;
>> +
>> + /*
>> + * Ensure that unique upper link is always the first item in the list.
>> + */
>> + if (unique)
>> + list_add_rcu(&upper->list, &dev->upper_dev_list);
>> + else
>> + list_add_tail_rcu(&upper->list, &dev->upper_dev_list);
>> + dev_hold(upper_dev);
>> +
>> + return 0;
>> +}
>> +/**
>> + * netdev_upper_dev_link - Add a link to the upper device
>> + * @dev: device
>> + * @upper_dev: new upper device
>> + *
>> + * Adds a link to device which is upper to this one. The caller must hold
>> + * the RTNL semaphore. On a failure a negative errno code is returned.
>> + * On success the reference counts are adjusted and the function
>> + * returns zero.
>> + */
>> +int netdev_upper_dev_link(struct net_device *dev,
>> + struct net_device *upper_dev)
>> +{
>> + return __netdev_upper_dev_link(dev, upper_dev, false);
>> +}
>> +EXPORT_SYMBOL(netdev_upper_dev_link);
>> +
>> +/**
>> + * netdev_unique_upper_dev_link - Add a unique link to the upper device
>> + * @dev: device
>> + * @upper_dev: new upper device
>> + *
>> + * Adds a link to device which is upper to this one. In this case, only
>> + * one unique upper device can be linked, although other non-unique devices
>> + * might be linked as well. The caller must hold the RTNL semaphore.
>> + * On a failure a negative errno code is returned. On success the reference
>> + * counts are adjusted and the function returns zero.
>> + */
>> +int netdev_unique_upper_dev_link(struct net_device *dev,
>> + struct net_device *upper_dev)
>> +{
>> + return __netdev_upper_dev_link(dev, upper_dev, true);
>> +}
>> +EXPORT_SYMBOL(netdev_unique_upper_dev_link);
>> +
>> +/**
>> + * netdev_upper_free_rcu - Frees a upper device list item via the RCU pointer
>> + * @entry: the entry's RCU field
>> + *
>> + * This function is designed to be used as a callback to the call_rcu()
>> + * function so that the memory allocated to the netdev upper device list item
>> + * can be released safely.
>> + */
>> +static void netdev_upper_free_rcu(struct rcu_head *entry)
>> +{
>> + struct netdev_upper *upper;
>> +
>> + upper = container_of(entry, struct netdev_upper, rcu);
>> + kfree(upper);
>> +}
>> +
>> +/**
>> + * netdev_upper_dev_unlink - Removes a link to upper device
>> + * @dev: device
>> + * @upper_dev: new upper device
>> + *
>> + * Removes a link to device which is upper to this one. The caller must hold
>> + * the RTNL semaphore.
>> + */
>> +void netdev_upper_dev_unlink(struct net_device *dev,
>> + struct net_device *upper_dev)
>> +{
>> + struct netdev_upper *upper;
>> +
>> + ASSERT_RTNL();
>> +
>> + upper = __netdev_find_upper(dev, upper_dev);
>> + if (!upper)
>> + return;
>> + list_del_rcu(&upper->list);
>> + dev_put(upper_dev);
>> + call_rcu(&upper->rcu, netdev_upper_free_rcu);
>> +}
>> +EXPORT_SYMBOL(netdev_upper_dev_unlink);
>> +
>> /**
>> * netdev_set_master - set up master pointer
>> * @slave: slave device
>> @@ -4438,19 +4661,23 @@ static int __init dev_proc_init(void)
>> int netdev_set_master(struct net_device *slave, struct net_device *master)
>> {
>> struct net_device *old = slave->master;
>> + int err;
>>
>> ASSERT_RTNL();
>>
>> if (master) {
>> if (old)
>> return -EBUSY;
>> - dev_hold(master);
>> + err = netdev_unique_upper_dev_link(slave, master);
>> + if (err)
>> + return err;
>> }
>>
>> slave->master = master;
>>
>> if (old)
>> - dev_put(old);
>> + netdev_upper_dev_unlink(slave, master);
>> +
>> return 0;
>> }
>> EXPORT_SYMBOL(netdev_set_master);
>> @@ -5999,6 +6226,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>> INIT_LIST_HEAD(&dev->napi_list);
>> INIT_LIST_HEAD(&dev->unreg_list);
>> INIT_LIST_HEAD(&dev->link_watch_list);
>> + INIT_LIST_HEAD(&dev->upper_dev_list);
>> dev->priv_flags = IFF_XMIT_DST_RELEASE;
>> setup(dev);
>>
>
^ permalink raw reply
* Re: [PATCH 03/19] netfilter: nf_nat_sip: fix via header translation with multiple parameters
From: Patrick McHardy @ 2012-08-14 12:23 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, netdev
In-Reply-To: <20120814002857.GC14483@1984>
On Tue, 14 Aug 2012, Pablo Neira Ayuso wrote:
> On Thu, Aug 09, 2012 at 10:08:47PM +0200, kaber@trash.net wrote:
>> From: Patrick McHardy <kaber@trash.net>
>>
>> Via-headers are parsed beginning at the first character after the Via-address.
>> When the address is translated first and its length decreases, the offset to
>> start parsing at is incorrect and header parameters might be missed.
>>
>> Update the offset after translating the Via-address to fix this.
>
> Applied, thanks Patrick.
Thanks Pablo. I'll submit the final IPv6-NAT patches once these three
patches have made it into net-next/nf-next.
^ permalink raw reply
* [PATCH 4/5] sungem: Call netif_carrier_off() after register_netdev()
From: Ilya Shchepetkov @ 2012-08-14 10:28 UTC (permalink / raw)
To: David S. Miller; +Cc: Ilya Shchepetkov, netdev, linux-kernel, ldv-project
In-Reply-To: <1344940135-17079-1-git-send-email-shchepetkov@ispras.ru>
For carrier detection to work properly when binding the driver with a
cable unplugged, netif_carrier_off() should be called after
register_netdev(), not before.
Calling netif_carrier_off() before register_netdev() was causing the
network interface to miss a linkwatch pending event leading to an
inconsistent state if the link is not up when interface is initialized.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Ilya Shchepetkov <shchepetkov@ispras.ru>
---
drivers/net/ethernet/sun/sungem.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
index 9ae12d0..c57c9f6 100644
--- a/drivers/net/ethernet/sun/sungem.c
+++ b/drivers/net/ethernet/sun/sungem.c
@@ -2909,7 +2909,6 @@ static int __devinit gem_init_one(struct pci_dev *pdev,
gp->lstate = link_down;
gp->timer_ticks = 0;
- netif_carrier_off(dev);
gp->regs = ioremap(gemreg_base, gemreg_len);
if (!gp->regs) {
@@ -2988,6 +2987,9 @@ static int __devinit gem_init_one(struct pci_dev *pdev,
goto err_out_free_consistent;
}
+ /* Turn off carrier */
+ netif_carrier_off(dev);
+
/* Undo the get_cell with appropriate locking (we could use
* ndo_init/uninit but that would be even more clumsy imho)
*/
--
1.7.7
^ permalink raw reply related
* [Patch] net: allow calling rtmsg_ifinfo() in atomic
From: Cong Wang @ 2012-08-14 10:28 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, John Fastabend, Greg Rose, Thomas Graf,
Eric Dumazet, Ben Hutchings, Cong Wang
(Against net tree.)
When running 'systemctl restart network.service', I got the following
warning:
[ 1123.199677]
[ 1123.310275] ===============================
[ 1123.442202] [ INFO: suspicious RCU usage. ]
[ 1123.558207] 3.6.0-rc1+ #109 Not tainted
[ 1123.665204] -------------------------------
[ 1123.768254] include/linux/rcupdate.h:430 Illegal context switch in RCU read-side critical section!
[ 1123.992320]
[ 1123.992320] other info that might help us debug this:
[ 1123.992320]
[ 1124.307382]
[ 1124.307382] rcu_scheduler_active = 1, debug_locks = 0
[ 1124.522220] 2 locks held by sysctl/5710:
[ 1124.648364] #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff81768498>] rtnl_trylock+0x15/0x17
[ 1124.882211] #1: (rcu_read_lock){.+.+.+}, at: [<ffffffff81871df8>] rcu_lock_acquire+0x0/0x29
[ 1125.085209]
[ 1125.085209] stack backtrace:
[ 1125.332213] Pid: 5710, comm: sysctl Not tainted 3.6.0-rc1+ #109
[ 1125.441291] Call Trace:
[ 1125.545281] [<ffffffff8109d915>] lockdep_rcu_suspicious+0x109/0x112
[ 1125.667212] [<ffffffff8107c240>] rcu_preempt_sleep_check+0x45/0x47
[ 1125.781838] [<ffffffff8107c260>] __might_sleep+0x1e/0x19b
[ 1125.900306] [<ffffffff8113e4a2>] slab_pre_alloc_hook+0x2d/0x38
[ 1126.012217] [<ffffffff817508f4>] ? __alloc_skb+0x4c/0x1a2
[ 1126.115156] [<ffffffff811411ea>] kmem_cache_alloc_node+0x2c/0x1c2
[ 1126.260520] [<ffffffff8109cae1>] ? lock_is_held+0x8a/0x95
[ 1126.391261] [<ffffffff817508f4>] ? __alloc_skb+0x4c/0x1a2
[ 1126.515221] [<ffffffff817508f4>] __alloc_skb+0x4c/0x1a2
[ 1126.615272] [<ffffffff8176778b>] ? if_nlmsg_size+0x164/0x19b
[ 1126.741293] [<ffffffff8176718e>] nlmsg_new+0x14/0x16
[ 1126.863267] [<ffffffff8176994d>] rtmsg_ifinfo+0x3b/0xcd
[ 1126.982196] [<ffffffff8109cae1>] ? lock_is_held+0x8a/0x95
[ 1127.102201] [<ffffffff81769a0f>] rtnetlink_event+0x30/0x34
[ 1127.229490] [<ffffffff8198a241>] notifier_call_chain+0x96/0xcd
[ 1127.338260] [<ffffffff810760b3>] raw_notifier_call_chain+0x14/0x16
[ 1127.445223] [<ffffffff81757ac5>] call_netdevice_notifiers+0x4a/0x4f
[ 1127.560195] [<ffffffff81757ffb>] netdev_features_change+0x16/0x18
[ 1127.668273] [<ffffffff8175e0ee>] netdev_update_features+0x20/0x25
[ 1127.772188] [<ffffffff8175e125>] dev_disable_lro+0x32/0x6b
[ 1127.885174] [<ffffffff81872d26>] dev_forward_change+0x30/0xcb
[ 1128.013214] [<ffffffff818738c4>] addrconf_forward_change+0x85/0xc5
[ 1128.118328] [<ffffffff818739bc>] addrconf_sysctl_forward+0xb8/0x108
[ 1128.278964] [<ffffffff81873904>] ? addrconf_forward_change+0xc5/0xc5
[ 1128.383230] [<ffffffff811af269>] proc_sys_call_handler+0x8a/0xb8
[ 1128.475407] [<ffffffff811af2ab>] proc_sys_write+0x14/0x16
[ 1128.578263] [<ffffffff81152a9e>] vfs_write+0xaf/0xf6
[ 1128.674728] [<ffffffff81153edf>] ? fget_light+0x3a/0xa1
[ 1128.790492] [<ffffffff81152c99>] sys_write+0x4d/0x74
[ 1128.902165] [<ffffffff8112040e>] ? do_anonymous_page+0x162/0x1d6
[ 1129.017203] [<ffffffff8198dea9>] system_call_fastpath+0x16/0x1b
This is due to that we call rtmsg_ifinfo() with RCU read lock held,
and because rtmsg_ifinfo() may block, this is invalid.
This patch fixes it by allowing callees to specify GFP when
calling it. In this case, netdev_features_change() calls it
with GFP_ATOMIC.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Greg Rose <gregory.v.rose@intel.com>
Cc: Thomas Graf <tgraf@suug.ch>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index db71c4a..8d5b0b0 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -621,7 +621,8 @@ extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics);
extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
u32 id, long expires, u32 error);
-extern void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change);
+extern void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change,
+ gfp_t flags);
/* RTNL is used as a global lock for all changes to network configuration */
extern void rtnl_lock(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index a39354e..8c44e4c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1089,6 +1089,7 @@ int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
void netdev_features_change(struct net_device *dev)
{
call_netdevice_notifiers(NETDEV_FEAT_CHANGE, dev);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_ATOMIC);
}
EXPORT_SYMBOL(netdev_features_change);
@@ -1104,7 +1105,7 @@ void netdev_state_change(struct net_device *dev)
{
if (dev->flags & IFF_UP) {
call_netdevice_notifiers(NETDEV_CHANGE, dev);
- rtmsg_ifinfo(RTM_NEWLINK, dev, 0);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
}
}
EXPORT_SYMBOL(netdev_state_change);
@@ -1204,7 +1205,7 @@ int dev_open(struct net_device *dev)
if (ret < 0)
return ret;
- rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL);
call_netdevice_notifiers(NETDEV_UP, dev);
return ret;
@@ -1277,7 +1278,7 @@ static int dev_close_many(struct list_head *head)
__dev_close_many(head);
list_for_each_entry(dev, head, unreg_list) {
- rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL);
call_netdevice_notifiers(NETDEV_DOWN, dev);
}
@@ -4482,7 +4483,7 @@ int netdev_set_bond_master(struct net_device *slave, struct net_device *master)
else
slave->flags &= ~IFF_SLAVE;
- rtmsg_ifinfo(RTM_NEWLINK, slave, IFF_SLAVE);
+ rtmsg_ifinfo(RTM_NEWLINK, slave, IFF_SLAVE, GFP_KERNEL);
return 0;
}
EXPORT_SYMBOL(netdev_set_bond_master);
@@ -4776,7 +4777,7 @@ int dev_change_flags(struct net_device *dev, unsigned int flags)
changes = old_flags ^ dev->flags;
if (changes)
- rtmsg_ifinfo(RTM_NEWLINK, dev, changes);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, changes, GFP_KERNEL);
__dev_notify_flags(dev, old_flags);
return ret;
@@ -5289,7 +5290,7 @@ static void rollback_registered_many(struct list_head *head)
if (!dev->rtnl_link_ops ||
dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U);
+ rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
/*
* Flush the unicast and multicast chains
@@ -5643,7 +5644,7 @@ int register_netdevice(struct net_device *dev)
*/
if (!dev->rtnl_link_ops ||
dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL);
out:
return ret;
@@ -6227,7 +6228,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
*/
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
call_netdevice_notifiers(NETDEV_UNREGISTER_BATCH, dev);
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U);
+ rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
/*
* Flush the unicast and multicast chains
@@ -6260,7 +6261,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
* Prevent userspace races by waiting until the network
* device is fully setup before sending notifications.
*/
- rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL);
synchronize_net();
err = 0;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 2c5a0a0..ab214ce 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1631,7 +1631,7 @@ int rtnl_configure_link(struct net_device *dev, const struct ifinfomsg *ifm)
}
dev->rtnl_link_state = RTNL_LINK_INITIALIZED;
- rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL);
__dev_notify_flags(dev, old_flags);
return 0;
@@ -1962,14 +1962,14 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change)
+void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change, gfp_t flags)
{
struct net *net = dev_net(dev);
struct sk_buff *skb;
int err = -ENOBUFS;
size_t if_info_size;
- skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), GFP_KERNEL);
+ skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), flags);
if (skb == NULL)
goto errout;
@@ -1980,7 +1980,7 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change)
kfree_skb(skb);
goto errout;
}
- rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
+ rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
return;
errout:
if (err < 0)
@@ -2359,9 +2359,10 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_UNREGISTER_BATCH:
case NETDEV_RELEASE:
case NETDEV_JOIN:
+ case NETDEV_FEAT_CHANGE:
break;
default:
- rtmsg_ifinfo(RTM_NEWLINK, dev, 0);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
break;
}
return NOTIFY_DONE;
^ permalink raw reply related
* [PATCH 5/5] net/hyperv: Call netif_carrier_off() after register_netdev()
From: Ilya Shchepetkov @ 2012-08-14 10:28 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: Ilya Shchepetkov, Haiyang Zhang, David S. Miller, devel, netdev,
linux-kernel, ldv-project
In-Reply-To: <1344940135-17079-1-git-send-email-shchepetkov@ispras.ru>
For carrier detection to work properly when binding the driver with a
cable unplugged, netif_carrier_off() should be called after
register_netdev(), not before.
Calling netif_carrier_off() before register_netdev() was causing the
network interface to miss a linkwatch pending event leading to an
inconsistent state if the link is not up when interface is initialized.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Ilya Shchepetkov <shchepetkov@ispras.ru>
---
drivers/net/hyperv/netvsc_drv.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 8c5a1c4..5734ad0 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -416,9 +416,6 @@ static int netvsc_probe(struct hv_device *dev,
if (!net)
return -ENOMEM;
- /* Set initial state */
- netif_carrier_off(net);
-
net_device_ctx = netdev_priv(net);
net_device_ctx->device_ctx = dev;
hv_set_drvdata(dev, net);
@@ -441,6 +438,9 @@ static int netvsc_probe(struct hv_device *dev,
goto out;
}
+ /* Set initial state */
+ netif_carrier_off(net);
+
/* Notify the netvsc driver of the new device */
device_info.ring_size = ring_size;
ret = rndis_filter_device_add(dev, &device_info);
--
1.7.7
^ permalink raw reply related
* [PATCH 3/5] net/mlx4_en: Call netif_carrier_off() after register_netdev()
From: Ilya Shchepetkov @ 2012-08-14 10:28 UTC (permalink / raw)
To: Yevgeny Petrilin
Cc: Ilya Shchepetkov, David S. Miller, Amir Vadai, Or Gerlitz,
Alexander Guller, netdev, linux-kernel, ldv-project
In-Reply-To: <1344940135-17079-1-git-send-email-shchepetkov@ispras.ru>
For carrier detection to work properly when binding the driver with a
cable unplugged, netif_carrier_off() should be called after
register_netdev(), not before.
Calling netif_carrier_off() before register_netdev() was causing the
network interface to miss a linkwatch pending event leading to an
inconsistent state if the link is not up when interface is initialized.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Ilya Shchepetkov <shchepetkov@ispras.ru>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index edd9cb8..7bf2923 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1676,13 +1676,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
mdev->pndev[port] = dev;
- netif_carrier_off(dev);
err = register_netdev(dev);
if (err) {
en_err(priv, "Netdev registration failed for port %d\n", port);
goto out;
}
priv->registered = 1;
+ netif_carrier_off(dev);
en_warn(priv, "Using %d TX rings\n", prof->tx_ring_num);
en_warn(priv, "Using %d RX rings\n", prof->rx_ring_num);
--
1.7.7
^ permalink raw reply related
* [PATCH 2/5] de2104x: Call netif_carrier_off() after register_netdev()
From: Ilya Shchepetkov @ 2012-08-14 10:28 UTC (permalink / raw)
To: Grant Grundler
Cc: Ilya Shchepetkov, David S. Miller, netdev, linux-kernel,
ldv-project
In-Reply-To: <1344940135-17079-1-git-send-email-shchepetkov@ispras.ru>
For carrier detection to work properly when binding the driver with a
cable unplugged, netif_carrier_off() should be called after
register_netdev(), not before.
Calling netif_carrier_off() before register_netdev() was causing the
network interface to miss a linkwatch pending event leading to an
inconsistent state if the link is not up when interface is initialized.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Ilya Shchepetkov <shchepetkov@ispras.ru>
---
drivers/net/ethernet/dec/tulip/de2104x.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c
index 61cc093..237f254 100644
--- a/drivers/net/ethernet/dec/tulip/de2104x.c
+++ b/drivers/net/ethernet/dec/tulip/de2104x.c
@@ -2005,8 +2005,6 @@ static int __devinit de_init_one (struct pci_dev *pdev,
de->media_timer.function = de21041_media_timer;
de->media_timer.data = (unsigned long) de;
- netif_carrier_off(dev);
-
/* wake up device, assign resources */
rc = pci_enable_device(pdev);
if (rc)
@@ -2074,6 +2072,9 @@ static int __devinit de_init_one (struct pci_dev *pdev,
rc = register_netdev(dev);
if (rc)
goto err_out_iomap;
+
+ /* turn off carrier */
+ netif_carrier_off(dev);
/* print info about board and interface just registered */
netdev_info(dev, "%s at %p, %pM, IRQ %d\n",
--
1.7.7
^ permalink raw reply related
* [PATCH 0/5] Call netif_carrier_off() after register_netdev()
From: Ilya Shchepetkov @ 2012-08-14 10:28 UTC (permalink / raw)
To: David S. Miller; +Cc: Ilya Shchepetkov, netdev, linux-kernel, ldv-project
Hi,
There are several patches on the subject:
31bde1ceaa873bcaecd49e829bfabceacc4c512d
c55ad8e56b983f03589b38b4504b5d1f41161ff8
e826eafa65c6f1f7c8db5a237556cebac57ebcc5
0d672e9f8ac320c6d1ea9103db6df7f99ea20361
6a3c869a6021f4abcd69aa5fbb15c63f69eb36fe
In 2008, David Miller wrote in his commit:
(b47300168e770b60ab96c8924854c3b0eb4260eb)
>net: Do not fire linkwatch events until the device is registered.
>Several device drivers try to do things like netif_carrier_off()
>before register_netdev() is invoked. This is bogus, but too many
>drivers do this to fix them all up in one go.
But I don't understand what will happen in this case?
Thanks,
Ilya
^ permalink raw reply
* Re: [patch net-next 01/16] net: introduce upper device lists
From: Jiri Pirko @ 2012-08-14 10:18 UTC (permalink / raw)
To: Cong Wang
Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
bhutchings, therbert, joe, gregory.v.rose, john.r.fastabend,
linux-rdma, linux-kernel, linux-s390, bridge, fbl
In-Reply-To: <502A1429.9060005@gmail.com>
Tue, Aug 14, 2012 at 11:02:33AM CEST, xiyou.wangcong@gmail.com wrote:
>On 08/13/2012 11:27 PM, Jiri Pirko wrote:
>>This lists are supposed to serve for storing pointers to all upper devices.
>>Eventually it will replace dev->master pointer which is used for
>>bonding, bridge, team but it cannot be used for vlan, macvlan where
>>there might be multiple "masters" present.
>>
>>New upper device list resolves this limitation. Also, the information
>>stored in lists is used for preventing looping setups like
>>"bond->somethingelse->samebond"
>
>Hi, Jiri,
>
>I have some difficulty to understand this patch description, so take
>bridged bonding as an example, IIUC, both the bridge dev and bonding
>dev will be in the upper dev list of eth0 and eth1? And br0 will be
>in the upper dev list of bond0 too?
No. Only direct upper device is in the list.
>
>I think it would be nice to describe the hierarchy after your patches.
Hierarchy is the same after.
>
>Thanks!
>
^ permalink raw reply
* Re: [PATCH] netvm: check for page == NULL when propogating the skb->pfmemalloc flag
From: Mel Gorman @ 2012-08-14 10:18 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: linux-mm, linux-kernel, netdev, xen-devel, konrad, Ian.Campbell,
David Miller, akpm
In-Reply-To: <50294DF0.8040206@goop.org>
On Mon, Aug 13, 2012 at 11:56:48AM -0700, Jeremy Fitzhardinge wrote:
> On 08/13/2012 03:47 AM, Mel Gorman wrote:
> > Resending to correct Jeremy's address.
> >
> > On Wed, Aug 08, 2012 at 03:50:46PM -0700, David Miller wrote:
> >> From: Mel Gorman <mgorman@suse.de>
> >> Date: Tue, 7 Aug 2012 09:55:55 +0100
> >>
> >>> Commit [c48a11c7: netvm: propagate page->pfmemalloc to skb] is responsible
> >>> for the following bug triggered by a xen network driver
> >> ...
> >>> The problem is that the xenfront driver is passing a NULL page to
> >>> __skb_fill_page_desc() which was unexpected. This patch checks that
> >>> there is a page before dereferencing.
> >>>
> >>> Reported-and-Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >>> Signed-off-by: Mel Gorman <mgorman@suse.de>
> >> That call to __skb_fill_page_desc() in xen-netfront.c looks completely bogus.
> >> It's the only driver passing NULL here.
> >>
> >> That whole song and dance figuring out what to do with the head
> >> fragment page, depending upon whether the length is greater than the
> >> RX_COPY_THRESHOLD, is completely unnecessary.
> >>
> >> Just use something like a call to __pskb_pull_tail(skb, len) and all
> >> that other crap around that area can simply be deleted.
> > I looked at this for a while but I did not see how __pskb_pull_tail()
> > could be used sensibly but I'm simily not familiar with writing network
> > device drivers or Xen.
> >
> > This messing with RX_COPY_THRESHOLD seems to be related to how the frontend
> > and backend communicate (maybe some fixed limitation of the xenbus). The
> > existing code looks like it is trying to take the fragments received and
> > pass them straight to the backend without copying by passing the fragments
> > to the backend without copying. I worry that if I try converting this to
> > __pskb_pull_tail() that it would either hit the limitation of xenbus or
> > introduce copying where it is not wanted.
> >
> > I'm going to have to punt this to Jeremy and the other Xen folk as I'm not
> > sure what the original intention was and I don't have a Xen setup anywhere
> > to test any patch. Jeremy, xen folk?
>
> It's been a while since I've looked at that stuff, but as I remember,
> the issue is that since the packet ring memory is shared with another
> domain which may be untrustworthy, we want to make copies of the headers
> before making any decisions based on them so that the other domain can't
> change them after header processing but before they're actually sent.
> (The packet payload is considered less important, but of course the same
> issue applies if you're using some kind of content-aware packet filter.)
>
> So that's the rationale for always copying RX_COPY_THRESHOLD, even if
> the packet is larger than that amount. As far as I know, changing this
> behaviour wouldn't break the ring protocol, but it does introduce a
> potential security issue.
>
David,
This leaves us somewhat in a pickle. If I'm reading this right (which I may
not be) it means that using __pskb_pull_tail() will work in the ideal case
but potentially introduces a subtle issue in the future. This bug could be
"fixed" in the driver by partially reverting [01c68026: xen: netfront:
convert to SKB paged frag API.]. That could be viewed as sweeping the
problem under the carpet but it does contain the problem to the xen-netfront
driver. A new helper could be created like __skb_clear_page_desc but that
is overkill for one driver and feels as ugly.
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [Xen-devel] [PATCH] netvm: check for page == NULL when propogating the skb->pfmemalloc flag
From: Mel Gorman @ 2012-08-14 10:05 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Miller, Ian Campbell, xen-devel, netdev, linux-kernel,
linux-mm, konrad, akpm
In-Reply-To: <20120813154144.GA24868@phenom.dumpdata.com>
On Mon, Aug 13, 2012 at 11:41:44AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Aug 08, 2012 at 03:50:46PM -0700, David Miller wrote:
> > From: Mel Gorman <mgorman@suse.de>
> > Date: Tue, 7 Aug 2012 09:55:55 +0100
> >
> > > Commit [c48a11c7: netvm: propagate page->pfmemalloc to skb] is responsible
> > > for the following bug triggered by a xen network driver
> > ...
> > > The problem is that the xenfront driver is passing a NULL page to
> > > __skb_fill_page_desc() which was unexpected. This patch checks that
> > > there is a page before dereferencing.
> > >
> > > Reported-and-Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > Signed-off-by: Mel Gorman <mgorman@suse.de>
> >
> > That call to __skb_fill_page_desc() in xen-netfront.c looks completely bogus.
> > It's the only driver passing NULL here.
>
> It looks to be passing a valid page pointer (at least by looking
> at the code) so I am not sure how it got turned in a NULL.
>
Are we looking at different code bases? I see this and I was assuming it
was the source of the bug.
__skb_fill_page_desc(skb, 0, NULL, 0, 0);
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [GIT PULL nf-next] IPVS
From: Pablo Neira Ayuso @ 2012-08-14 9:06 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer
In-Reply-To: <1344562895-22790-1-git-send-email-horms@verge.net.au>
On Fri, Aug 10, 2012 at 10:41:29AM +0900, Simon Horman wrote:
> Hi Pablo,
>
> please consider the following enhancements to IPVS for inclusion in 3.7.
>
> ----------------------------------------------------------------
> The following changes since commit 173f8654746c138a08f51a8a0db7747763a896a2:
>
> Merge tag 'ext4_for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4 (2012-07-27 20:52:25 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git master
Pulled, thanks Simon.
^ permalink raw reply
* Re: [patch net-next 01/16] net: introduce upper device lists
From: Cong Wang @ 2012-08-14 9:02 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
edumazet-hpIqsD4AKlfQT0dZR+AlfA,
faisal.latif-ral2JQCrhuEAvxtiuMwx3w,
roland-DgEjT+Ai2ygdnm+yROfE0A, sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
fubar-r/Jw6+rmf7HQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
divy-ut6Up61K2wZBDgjK7y7TUQ,
jitendra.kalsaria-h88ZbnxC6KDQT0dZR+AlfA,
sony.chacko-h88ZbnxC6KDQT0dZR+AlfA,
linux-driver-h88ZbnxC6KDQT0dZR+AlfA, kaber-dcUjhNyLwpNeoWH0uzbU5w,
ursula.braun-tA70FqPdS9bQT0dZR+AlfA,
blaschka-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
linux390-tA70FqPdS9bQT0dZR+AlfA,
shemminger-ZtmgI6mnKB3QT0dZR+AlfA,
bhutchings-s/n/eUQHGBpZroRs9YW3xA,
therbert-hpIqsD4AKlfQT0dZR+AlfA, joe-6d6DIl74uiNBDgjK7y7TUQ,
gregory.v.rose-ral2JQCrhuEAvxtiuMwx3w,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA,
bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
fbl-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1344871635-1052-2-git-send-email-jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
On 08/13/2012 11:27 PM, Jiri Pirko wrote:
> This lists are supposed to serve for storing pointers to all upper devices.
> Eventually it will replace dev->master pointer which is used for
> bonding, bridge, team but it cannot be used for vlan, macvlan where
> there might be multiple "masters" present.
>
> New upper device list resolves this limitation. Also, the information
> stored in lists is used for preventing looping setups like
> "bond->somethingelse->samebond"
Hi, Jiri,
I have some difficulty to understand this patch description, so take
bridged bonding as an example, IIUC, both the bridge dev and bonding dev
will be in the upper dev list of eth0 and eth1? And br0 will be in the
upper dev list of bond0 too?
I think it would be nice to describe the hierarchy after your patches.
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: [RFC PATCH 0/2] net: connect to UNIX sockets from specified root
From: Stanislav Kinsbursky @ 2012-08-14 8:46 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Pavel Emelianov, H. Peter Anvin, Alan Cox,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
tim.c.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org
In-Reply-To: <20120813182431.GA4234-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
13.08.2012 22:24, J. Bruce Fields пишет:
> On Mon, Aug 13, 2012 at 09:39:53PM +0400, Stanislav Kinsbursky wrote:
>> 13.08.2012 20:47, J. Bruce Fields пишет:
>>> On Sat, Aug 11, 2012 at 03:15:24PM +0400, Stanislav Kinsbursky wrote:
>>>> 11.08.2012 10:23, Pavel Emelyanov пишет:
>>>>> On 08/11/2012 03:09 AM, H. Peter Anvin wrote:
>>>>>> On 08/10/2012 12:28 PM, Alan Cox wrote:
>>>>>>> Explicitly for Linux yes - this is not generally true of the
>>>>>>> AF_UNIX socket domain and even the permissions aspect isn't
>>>>>>> guaranteed to be supported on some BSD environments !
>>>>>> Yes, but let's worry about what the Linux behavior should be.
>>>>>>
>>>>>>> The name is however just a proxy for the socket itself. You
>>>>>>> don't even get a device node in the usual sense or the same inode
>>>>>>> in the file system space.
>>>>>> No, but it is looked up the same way any other inode is (the
>>>>>> difference between FIFOs and sockets is that sockets have separate
>>>>>> connections, which is also why open() on sockets would be nice.)
>>>>>>
>>>>>> However, there is a fundamental difference between AF_UNIX sockets
>>>>>> and open(), and that is how the pathname is delivered. It thus
>>>>>> would make more sense to provide the openat()-like information in
>>>>>> struct sockaddr_un, but that may be very hard to do in a sensible
>>>>>> way. In that sense it perhaps would be cleaner to be able to do
>>>>>> an open[at]() on the socket node with O_PATH (perhaps there should
>>>>>> be an O_SOCKET option, even?) and pass the resulting file
>>>>>> descriptor to bind() or connect().
>>>>> I vote for this (openat + O_WHATEVER on a unix socket) as well. It
>>>>> will help us in checkpoint-restore, making handling of
>>>>> overmounted/unlinked sockets much cleaner.
>>>> I have to notice, that it's not enough and doesn't solve the issue.
>>>> There should be some way how to connect/bind already existent unix
>>>> socket (from kernel, at least), because socket can be created in user
>>>> space. And this way (sock operation or whatever) have to provide an
>>>> ability to lookup UNIX socket starting from specified root to support
>>>> containers.
>>> I don't understand--the rpcbind sockets are created by the kernel. What
>>> am I missing?
>>
>> Kernel preform connect to rpcbind socket (i.e. user-space binds it),
>> doesn't it?
>
> I'm confused, possibly because there are three "sockets" here: the
> client-side socket that's connected, the server-side socket that's bound,
> and the common object that exists in the filesystem namespace.
>
> Userland creates the server-side socket and binds to it. All of that is
> done in the context of the rpcbind process, so is created in rpcbind's
> namespace. That should be OK, right?
>
> The client side socket is created and connected in xs_local_setup_socket().
>
> Making sure they both end up with the same thing is a matter of making sure
> they lookup the same path in the same namespace. The difficult part of that
> is the in-kernel client-side socket connect, where we don't have the right
> process context any more.
>
Looks like I'm missing something important.
Where are these UNIX in-kernel created and listening sockets (in code, I mean)?
--
Best regards,
Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 V2 09/12] net/eipoib: Add main driver functionality
From: Or Gerlitz @ 2012-08-14 8:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eric W. Biederman, davem, roland, netdev, ali, sean.hefty,
Erez Shitrit, Doug Ledford
In-Reply-To: <20120812205457.GA14081@redhat.com>
On 12/08/2012 23:54, Michael S. Tsirkin wrote:
> On Sun, Aug 12, 2012 at 05:13:43PM +0300, Or Gerlitz wrote:
>> On 12/08/2012 16:55, Michael S. Tsirkin wrote:
>>> I didn't realize you do ARP snooping. Why? I know you mangle
>>> outgoing ARP packets,
>>
>> Maybe I wasn't accurate/clear, we do mangle outgoing/incoming ARP
>> packets, from/to Ethernet ARPs to/from IPoIB ARPs.
>>
>>
>>> this will go away if you maintain a mapping in SM accessible to all guests.
>>
>> guests don't interact with IB, I assume you referred to dom0 code, eIPoIB or
>> another driver in the host. But what mapping exactly?
>
> Well we are getting into protocol design here.
wait... reading your responses again, I realized that we 1st and most
have to (try and) agree on the
problem statement before going/jumping to solutions.
AFAIU your email/s you maybe think that we mandate the admin to set a
specific MAC to the VM which is derived from the LID/QPN of the IPoIB
VIF serving it, well this is wrong, we don't, OTOH, indeed, the VM
source mac isn't sent on the wire, since the Ethernet header is dropped,
and on the receiving side is constructed from the LID/QPN
the IB packet arrived from, see next.
This reconstruction of what we call the REMAC (remote ethernet mac) is
based in the current submission on
the LID/QPN, and as I said earlier on this thread, we are revisiting
this approach -- where your idea below sounds
good: the eipoib driver can register with the SA an IB "service record"
entry mapping from LID/QPN to the VM mac, when ever a VM is to be served
by this eipoib instance, and remove the entry when the VM shouldn't be
served any more. This will allow to preserve 1:1 the Ethernet MAC header
sent by VMs on the receiving side.
> So here's a sketch showing how you could build a protocol that does work. But note it is not *exactly* IPoIB.
HOWEVER, this doesn't touch the IPoIB wire protocol, and hence on the
wire it IS exactly IPoIB. We only make use of your lovely suggestion to
apply this SA assistance, so the change doesn't involve
hardware/firmware nor the wire protocol.
Or.
> It is I think close enough that you can use existing NIC hardware/firmware, which is why it differs slightly from what Eric described, and is more complex. But it still shares the same property of no hacks, no packet snooping in driver, etc.
>
>
> And if you want to go that route, you really should talk to some IB
> protocol people to figure out what works, write a spec and try to
> standardize. lkml/netdev is not the right place.
>
> But since you asked, if I had to, I would probably try to
> do it like this:
>
> - Each device registers with the SA specifying the
> mac address (+ vlan?), SA stores the translation from that
> to IPoIB address.
> - alternatively, SA admin configures the translation statically
> - you get a packet with 6 byte mac address,
> query the SA for a mapping to IPoIB address, strip
> ethernet frame and send
> - multicast GID addresses can be similar, filled either when registering
> for multicast or by SA admin
>
> I think it's possible that you could also convert a mac address to
> EUI-64 and prepend a prefix to get a legal GID. But maybe I'm missing
> something. This could be handy for multicast.
>
>
> In both cases:
> - SA could return GID that you then resolve to
> LID using another query, or it could return LID so you save a roundtrip
> - results can be cached locally
> - SA can send updates when translation changes to flush this cache
>
>
> Now above means protocols such as ARP and DHCP use 48 bit addresses so
> you can not mix this new protocol with IPoIB. Maybe IPoIB could simply
> ignore irrelevant packets, but it's best not to try, get a
> different all-broadcast group and CM ID instead to avoid confusion.
>
> One other interesting thing you can do is forward multicast
> registration data from the router, translate to mgid
> by the SA and do appropriate IB mcast registrations.
>
>
^ permalink raw reply
* Re: [PATCH 13/21] userns: Teach inet_diag to work with user namespaces
From: Pavel Emelyanov @ 2012-08-14 8:35 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Vagin, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Miller
In-Reply-To: <1344889115-21610-13-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 08/14/2012 12:18 AM, Eric W. Biederman wrote:
> From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
>
> Compute the user namespace of the socket that we are replying to
> and translate the kuids of reported sockets into that user namespace.
>
> Cc: Andrew Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Acked-by: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
^ permalink raw reply
* [PATCH] net: Checking usb_register() return value
From: Marina Makienko @ 2012-08-14 8:11 UTC (permalink / raw)
To: Kalle Valo
Cc: Marina Makienko, John W. Linville, linux-wireless, netdev,
linux-kernel, ldv-project
ath6kl_usb_init() does not check usb_register() return value.
As a result it may incorrectly report success of driver initialization.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Marina Makienko <makienko@ispras.ru>
---
drivers/net/wireless/ath/ath6kl/usb.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c
index 3740c3d..39f05da 100644
--- a/drivers/net/wireless/ath/ath6kl/usb.c
+++ b/drivers/net/wireless/ath/ath6kl/usb.c
@@ -1196,7 +1196,11 @@ static struct usb_driver ath6kl_usb_driver = {
static int ath6kl_usb_init(void)
{
- usb_register(&ath6kl_usb_driver);
+ int ret = usb_register(&ath6kl_usb_driver);
+ if (ret) {
+ ath6kl_err("usb_register() failed: %d\n", ret);
+ return ret;
+ }
return 0;
}
--
1.7.7
^ permalink raw reply related
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Or Gerlitz @ 2012-08-14 7:41 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eric W. Biederman, davem, roland, netdev, ali, sean.hefty,
Erez Shitrit, Doug Ledford
In-Reply-To: <20120812135544.GB6003@redhat.com>
On 12/08/2012 13:22, Michael S. Tsirkin wrote:
>>>>>>> - IGMP/MAC snooping in a driver is just too hairy.
>>>>
>>>>> mmm, any rough idea/direction how to do that otherwise?
>>> Sure, even two ways, ideally you'd do both:)
>>> A. fix macvtap
>>> 1. Use netdev_for_each_mc_addr etc to get multicast addresses
>>> 2. teach macvtap to fill that in (it currently floods multicasts
>>> for guest to guest communication so we ned to fix it anyway)
>>>
>>> B. fix bridge
>>> teach bridge to work for VMs without using promisc mode
>>
OK, I think I'm with you now... you suggest to avoid our direction of
implementing promiscuous multicast mode which is applied by today's
bridge, macvtap and friends by fixing these elements to support non
promisc multicast mode, yep, sure, sounds as win/win, which will
eliminate the need to do IGMP snooping in the driver.
Or.
^ permalink raw reply
* Re: [PATCH v2] net-tcp: TCP/IP stack bypass for loopback connections
From: David Miller @ 2012-08-14 7:37 UTC (permalink / raw)
To: billfink; +Cc: brutus, edumazet, netdev
In-Reply-To: <20120814023155.ffe9c01d.billfink@mindspring.com>
From: Bill Fink <billfink@mindspring.com>
Date: Tue, 14 Aug 2012 02:31:55 -0400
> I also believe it should be disabled by default, as that is
> the current behavior, and those who would gain an advantage
> from using it can easily enable it.
It benefits basically everyone, it makes things orders of
magnitude faster.
> While this could be a very useful feature in some environments,
> it seems to me it would be safest to have it disabled by default.
I violently disagree, and there is no way I'm having this
thing off by default.
^ permalink raw reply
* Re: [patch net-next 03/16] vlan: add link to upper device
From: Jiri Pirko @ 2012-08-14 7:24 UTC (permalink / raw)
To: Flavio Leitner
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
edumazet-hpIqsD4AKlfQT0dZR+AlfA,
faisal.latif-ral2JQCrhuEAvxtiuMwx3w,
roland-DgEjT+Ai2ygdnm+yROfE0A, sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
fubar-r/Jw6+rmf7HQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
divy-ut6Up61K2wZBDgjK7y7TUQ,
jitendra.kalsaria-h88ZbnxC6KDQT0dZR+AlfA,
sony.chacko-h88ZbnxC6KDQT0dZR+AlfA,
linux-driver-h88ZbnxC6KDQT0dZR+AlfA, kaber-dcUjhNyLwpNeoWH0uzbU5w,
ursula.braun-tA70FqPdS9bQT0dZR+AlfA,
blaschka-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
linux390-tA70FqPdS9bQT0dZR+AlfA,
shemminger-ZtmgI6mnKB3QT0dZR+AlfA,
bhutchings-s/n/eUQHGBpZroRs9YW3xA,
therbert-hpIqsD4AKlfQT0dZR+AlfA,
xiyou.wangcong-Re5JQEeQqe8AvxtiuMwx3w, joe-6d6DIl74uiNBDgjK7y7TUQ,
gregory.v.rose-ral2JQCrhuEAvxtiuMwx3w,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA,
bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
In-Reply-To: <20120813160421.77a74e4e-8Luz0qD3JK/tRgLqZ5aouw@public.gmane.org>
Mon, Aug 13, 2012 at 09:04:21PM CEST, fbl-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
>On Mon, 13 Aug 2012 17:27:02 +0200
>Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org> wrote:
>
>> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>> ---
>> net/8021q/vlan.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
>> index 9096bcb..739665e 100644
>> --- a/net/8021q/vlan.c
>> +++ b/net/8021q/vlan.c
>> @@ -105,6 +105,8 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
>> */
>> unregister_netdevice_queue(dev, head);
>>
>> + netdev_upper_dev_unlink(real_dev, dev);
>> +
>> if (grp->nr_vlan_devs == 0)
>> vlan_gvrp_uninit_applicant(real_dev);
>>
>> @@ -162,9 +164,13 @@ int register_vlan_dev(struct net_device *dev)
>> if (err < 0)
>> goto out_uninit_applicant;
>>
>> + err = netdev_upper_dev_link(real_dev, dev);
>> + if (err)
>> + goto out_uninit_applicant;
>> +
>> err = register_netdevice(dev);
>> if (err < 0)
>> - goto out_uninit_applicant;
>> + goto out_upper_dev_unlink;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>see below:
>
>>
>> /* Account for reference in struct vlan_dev_priv */
>> dev_hold(real_dev);
>> @@ -180,6 +186,8 @@ int register_vlan_dev(struct net_device *dev)
>>
>> return 0;
>>
>> +upper_dev_unlink:
>^^^^^^^^^^^^^^^^^^^
>should be out_upper_dev_unlink:
fixed.
Thanks!
>
>fbl
>
>> + netdev_upper_dev_unlink(real_dev, dev);
>> out_uninit_applicant:
>> if (grp->nr_vlan_devs == 0)
>> vlan_gvrp_uninit_applicant(real_dev);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 v2] net-tcp: TCP/IP stack bypass for loopback connections
From: Bill Fink @ 2012-08-14 6:31 UTC (permalink / raw)
To: Bruce "Brutus" Curtis; +Cc: David S. Miller, Eric Dumazet, netdev
In-Reply-To: <1344559958-29162-1-git-send-email-brutus@google.com>
On Thu, 9 Aug 2012, Bruce "Brutus" Curtis wrote:
> From: "Bruce \"Brutus\" Curtis" <brutus@google.com>
>
> TCP/IP loopback socket pair stack bypass, based on an idea by, and
> rough upstream patch from, David Miller <davem@davemloft.net> called
> "friends", the data structure modifcations and connection scheme are
> reused with extensive data-path changes.
>
> A new sysctl, net.ipv4.tcp_friends, is added:
> 0: disable friends and use the stock data path.
> 1: enable friends and bypass the stack data path, the default.
The following is from a user perspective, since I am not
intimately familiar with the internals of the TCP stack.
I think tcp_friends is a poor name from a user POV.
Something like tcp_bypass would be much better.
I also believe it should be disabled by default, as that is
the current behavior, and those who would gain an advantage
from using it can easily enable it.
Changing the behavior would violate the principle of least
surprise. Loopback TCP testing of an application or system
is often a useful first step in evaluating its behavior and
performance. If the TCP stack is bypassed, it will give a
very false impression when such tests are performed.
Does it preserve all TCP semantics for applications, including
things like urgent data, ancillary data, and TCP socket options
and ioctls. If it doesn't, it shouldn't be the default, and it
should be documented what features do and don't work when
tcp_bypass is enabled. If all TCP semantics are unchanged,
that would also be good to know and document.
And there's the already mentioned issue of breaking tcpdump
and related tools.
While this could be a very useful feature in some environments,
it seems to me it would be safest to have it disabled by default.
-Bill
> Note, when friends is enabled any loopback interpose, e.g. tcpdump,
> will only see the TCP/IP packets during connection establishment and
> finish, all data bypasses the stack and instead is delivered to the
> destination socket directly.
>
> Testing done on a 4 socket 2.2GHz "Quad-Core AMD Opteron(tm) Processor
> 8354 CPU" based system, netperf results for a single connection show
> increased TCP_STREAM throughput, increased TCP_RR and TCP_CRR transaction
> rate for most message sizes vs baseline and comparable to AF_UNIX.
>
> Significant increase (up to 4.88x) in aggregate throughput for multiple
> netperf runs (STREAM 32KB I/O x N) is seen.
^ permalink raw reply
* Re: [PATCH v5 00/15] some netpoll and netconsole fixes
From: David Miller @ 2012-08-14 4:01 UTC (permalink / raw)
To: amwang; +Cc: netdev
In-Reply-To: <1344851127.22116.5.camel@cr0>
From: Cong Wang <amwang@redhat.com>
Date: Mon, 13 Aug 2012 17:45:27 +0800
> Does this patchset look good to you now?
I'm backlogged and haven't had a chance to look at it yet.
^ 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