* [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
* [PATCH] TCP/IP stack bypass for loopback connections fix
From: Weiping Pan @ 2012-08-14 3:12 UTC (permalink / raw)
To: netdev; +Cc: brutus, Weiping Pan
In-Reply-To: <1344559958-29162-1-git-send-email-brutus@google.com>
I found two problems then I tested Bruce Curtis <brutus@google.com> loopback
patch, http://patchwork.ozlabs.org/patch/176304/
1 ‘friends’ undeclared in function ‘tcp_recvmsg’
I think it should use sysctl_tcp_friends
2 scheduling while atomic
BUG: scheduling while atomic: netperf/3820/0x10000200
Modules linked in: bridge stp llc autofs4 sunrpc ipv6 uinput
iTCO_wdt iTCO_vendor_suppo]
Pid: 3820, comm: netperf Tainted: G W 3.5.0+ #11
Call Trace:
[<ffffffff810861e2>] __schedule_bug+0x52/0x60
[<ffffffff8151e7de>] __schedule+0x68e/0x710
[<ffffffff8108a80a>] __cond_resched+0x2a/0x40
[<ffffffff8151e8f0>] _cond_resched+0x30/0x40
[<ffffffff8149b68e>] tcp_sendmsg+0x96e/0x12c0
[<ffffffff814bf928>] inet_sendmsg+0x48/0xb0
[<ffffffff812081a3>] ? selinux_socket_sendmsg+0x23/0x30
[<ffffffff8143c4be>] sock_sendmsg+0xbe/0xf0
[<ffffffff81078958>] ? finish_wait+0x68/0x90
[<ffffffff81172660>] ? fget_light+0x50/0xc0
[<ffffffff8143c629>] sys_sendto+0x139/0x190
[<ffffffff810d21cc>] ? __audit_syscall_entry+0xcc/0x210
[<ffffffff810d20a6>] ? __audit_syscall_exit+0x3d6/0x430
[<ffffffff81527c29>] system_call_fastpath+0x16/0x1b
The reason is that in tcp_friend_tail(), it holds a spinlock,
spin_lock_bh(&friend->sk_lock.slock);
but skb_add_data_nocache() can cause schedule, so this warning happends.
Wit this patch, the original patch can apply on top of net-next tree
commit 79cda75a107da(fib: use __fls() on non null argument),
and I test it works fine on x86_64 system.
Signed-off-by: Weiping Pan <wpan@redhat.com>
---
net/ipv4/tcp.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 6dc267c..bab15da 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -628,7 +628,7 @@ static inline struct sk_buff *tcp_friend_tail(struct sock *sk, int *copy)
int sz = 0;
if (skb_peek_tail(&friend->sk_receive_queue)) {
- spin_lock_bh(&friend->sk_lock.slock);
+ lock_sock(friend);
skb = skb_peek_tail(&friend->sk_receive_queue);
if (skb && skb->friend) {
if (!*copy)
@@ -637,7 +637,7 @@ static inline struct sk_buff *tcp_friend_tail(struct sock *sk, int *copy)
sz = *copy - skb->len;
}
if (!skb || sz <= 0)
- spin_unlock_bh(&friend->sk_lock.slock);
+ release_sock(friend);
}
*copy = sz;
@@ -655,7 +655,7 @@ static inline void tcp_friend_seq(struct sock *sk, int copy, int charge)
}
tp->rcv_nxt += copy;
tp->rcv_wup += copy;
- spin_unlock_bh(&friend->sk_lock.slock);
+ release_sock(friend);
friend->sk_data_ready(friend, copy);
@@ -676,7 +676,7 @@ static inline int tcp_friend_push(struct sock *sk, struct sk_buff *skb)
return -ECONNRESET;
}
- spin_lock_bh(&friend->sk_lock.slock);
+ lock_sock(friend);
skb->friend = sk;
skb_set_owner_r(skb, friend);
__skb_queue_tail(&friend->sk_receive_queue, skb);
@@ -1505,7 +1505,7 @@ out:
do_fault:
if (friend_tail)
- spin_unlock_bh(&friend->sk_lock.slock);
+ release_sock(friend);
else if (!skb->len) {
if (friend)
__kfree_skb(skb);
@@ -1932,7 +1932,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
available = TCP_SKB_CB(skb)->seq + skb->len - (*seq);
if ((available < target) &&
(len > sysctl_tcp_dma_copybreak) && !(flags & MSG_PEEK) &&
- !sysctl_tcp_low_latency && !friends &&
+ !sysctl_tcp_low_latency && !sysctl_tcp_friends &&
net_dma_find_channel()) {
preempt_enable_no_resched();
tp->ucopy.pinned_list =
--
1.7.1
^ permalink raw reply related
* Re: [net PATCH v1 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: John Fastabend @ 2012-08-14 3:00 UTC (permalink / raw)
To: Al Viro; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814023128.GG23464@ZenIV.linux.org.uk>
On 8/13/2012 7:31 PM, Al Viro wrote:
> On Mon, Aug 13, 2012 at 06:39:45PM -0700, John Fastabend wrote:
>> /* Bump the usage count and install the file. */
>> get_file(fp[i]);
>> fd_install(new_fd, fp[i]);
>> + sock = sock_from_file(fp[i], &err);
>> + if (sock)
>> + sock->sk->sk_cgrp_prioidx = prioidx;
>
> Better to do that before fd_install() exposes the sucker; it's not
> a serious issue, but it's more obviously correct that way.
>
Right. Moved it in v2.
^ permalink raw reply
* Re: [net PATCH v1 1/2] net: netprio: fix files lock and remove useless d_path bits
From: John Fastabend @ 2012-08-14 2:59 UTC (permalink / raw)
To: Al Viro; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814022951.GF23464@ZenIV.linux.org.uk>
On 8/13/2012 7:29 PM, Al Viro wrote:
> On Mon, Aug 13, 2012 at 06:39:39PM -0700, John Fastabend wrote:
>> - path = d_path(&file->f_path, tmp, PAGE_SIZE);
>
> What's using tmp after that patch and do you still need to bother
> allocating it?
>
nothing removed it in v2.
^ permalink raw reply
* [net PATCH v2 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: John Fastabend @ 2012-08-14 2:43 UTC (permalink / raw)
To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe
In-Reply-To: <20120814024320.6983.92222.stgit@jf-dev1-dcblab>
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;
fd_install(new_fd, fp[i]);
}
^ permalink raw reply related
* [net PATCH v2 1/2] net: netprio: fix files lock and remove useless d_path bits
From: John Fastabend @ 2012-08-14 2:43 UTC (permalink / raw)
To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe
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[] = {
^ permalink raw reply related
* Re: [net PATCH v1 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: Al Viro @ 2012-08-14 2:31 UTC (permalink / raw)
To: John Fastabend; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814013944.3144.53854.stgit@jf-dev1-dcblab>
On Mon, Aug 13, 2012 at 06:39:45PM -0700, John Fastabend wrote:
> /* Bump the usage count and install the file. */
> get_file(fp[i]);
> fd_install(new_fd, fp[i]);
> + sock = sock_from_file(fp[i], &err);
> + if (sock)
> + sock->sk->sk_cgrp_prioidx = prioidx;
Better to do that before fd_install() exposes the sucker; it's not
a serious issue, but it's more obviously correct that way.
^ permalink raw reply
* Re: [net PATCH v1 1/2] net: netprio: fix files lock and remove useless d_path bits
From: Al Viro @ 2012-08-14 2:29 UTC (permalink / raw)
To: John Fastabend; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814013939.3144.13167.stgit@jf-dev1-dcblab>
On Mon, Aug 13, 2012 at 06:39:39PM -0700, John Fastabend wrote:
> - path = d_path(&file->f_path, tmp, PAGE_SIZE);
What's using tmp after that patch and do you still need to bother
allocating it?
^ 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