* Re: [PATCH 1/2] siw: Fix ib_register_device() for > v2.6.34 kernels
From: Bernard Metzler @ 2010-09-23 15:26 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Andi Keen, David Miller, linux-kernel, linux-rdma,
linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA, Nicholas Bellinger,
netdev, Roland Dreier, Matthew Wilcox
In-Reply-To: <1285187417-10928-1-git-send-email-nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
Thanks, Nicholas. Just applied your patch.
Bernard.
linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org wrote on 09/22/2010 10:30:17 PM:
> From: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
>
> This patch adds a LINUX_VERSION_CODE > v2.6.34 check inside of
> siw_main.c:siw_register_device()
> around the use of ib_register_device(). In post v2.6.34 kernels
> this function accepts a second
> parameter used a sysfs port callback described here:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;
> a=commitdiff;h=9a6edb60ec10d86b1025a0cdad68fd89f1ddaf02
>
> This patch currently sets this second parameter to NULL.
>
> Signed-off-by: Nicholas A. Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
> ---
> softiwarp/siw_main.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/softiwarp/siw_main.c b/softiwarp/siw_main.c
> index cacedea..c97adee 100644
> --- a/softiwarp/siw_main.c
> +++ b/softiwarp/siw_main.c
> @@ -233,8 +233,11 @@ int siw_register_device(struct siw_dev *dev)
> ibdev->iwcm->add_ref = siw_qp_get_ref;
> ibdev->iwcm->rem_ref = siw_qp_put_ref;
> ibdev->iwcm->get_qp = siw_get_ofaqp;
> -
> +#if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 34)
> + rv = ib_register_device(ibdev, NULL);
> +#else
> rv = ib_register_device(ibdev);
> +#endif
> if (rv) {
> dprint(DBG_DM|DBG_ON, "(dev=%s): "
> "ib_register_device failed: rv=%d\n", ibdev->name, rv);
> --
> 1.5.6.5
>
> --
> 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
--
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: software iwarp stack update
From: Bernard Metzler @ 2010-09-23 15:25 UTC (permalink / raw)
To: Steve Wise
Cc: Andi Kleen, David Miller, linux-rdma, linux-rdma-owner,
Matthew Wilcox, Nicholas A. Bellinger, netdev
In-Reply-To: <4C9A6A2A.2020509@opengridcomputing.com>
linux-rdma-owner@vger.kernel.org wrote on 09/22/2010 10:42:18 PM:
> On 09/22/2010 03:35 PM, Nicholas A. Bellinger wrote:
> > On Wed, 2010-09-22 at 10:19 +0200, Bernard Metzler wrote:
> >
> >> Earlier this year, we announced the availability of an open source,
> >> full software implementation of the iWARP RDMA protocol stack - see
> >> my email "software iwarp stack" from March 14th at the linux-rdma list
> >> (http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg02940.html)
> >> While since then working on performance and stability, we provided
> >> some source code updates. Current user and kernel code is available at
> >> gitorious.org/softiwarp. Please see the CHANGES file in the
> >> kernel/ directory for a summary of the most recent changes.
> >>
> >> For more convenient testing, the latest update now allows for a
> >> stand-alone build of the kernel module without full kernel source
> >> code access. We tested the code with kernel version 2.6.34. If
> >> you are interested in a full software RDMA stack on Ethernet,
> >> please try it out.
> >>
> >> In the hope of providing useful information, I put
> >> netdev@vger.kernel.org on copy. Subscribers of this list,
> >> please put me on private cc in case you reply or comment, since
> >> I am not subscribed to the list.
> >> We would be more than happy if you netdev folks would consider
> >> a hardware independent RDMA kernel service as something useful and
> >> potentially to be integrated into the mainline network stack.
> >>
> >> Why might it be useful?
> >> A software RDMA stack makes the semantic advantages of
> >> asynchronous and one-sided communication available while obsoleting
> >> the need to deploy dedicated RDMA hardware or any protocol offloading
> >> (while not matching the lowest delay numbers of real RDMA hardware).
> >> Implementing the IETF's iWARP protocol stack on top of TCP kernel
> >> sockets, softiwarp integrates with the open fabrics environment
> >> and thus exports the RDMA kernel and user verbs interface.
> >>
> >> The efficiency of the Linux TCP/IP network stack together with
intrinsic
> >> advantages of the RDMA communication model (async. posting of work
> >> and reaping of work completions, transfer of send buffer ownership
> >> to the kernel which enables zero copy transmit, peer data placement
> >> without application scheduling, one-sided remote read operations etc.)
> >> can result in improved application-to-application performance and
> >> less CPU load, while using the unchanged kernel TCP stack.
> >>
> >> A software RDMA stack might promote wider RDMA deployment,
> >> since when using the host TCP stack, it enables RDMA semantic
> >> independent of dedicated hardware. softiwarp peers with real
> >> RNICs (tested with Chelsio's T3 adapter).
> >>
> >> softiwarp is still work in progress and we are very thankful for any
> >> suggestions/comments/bug reports. Please advise how we should proceed
> >> to bring the stack further to your attention. Would it be useful to
> >> provide patches against the current stable kernel version or the next
> >> release candidate?
> >>
> >>
> > Hi Bernard,
> >
> > So what I would recommend doing here to make things more appealing to
> > DaveM and other interested NetDev folks would be to clone a seperate
> > tree from the net-2.6.git or net-next-2.6.git repositories and include
> > the softiwarp/kernel.git code into a fresh 'in-kernel' clone tracking
> > the latest netdev code, and then keep git rebase'ing against DaveM's
> > last changes and update your local tree to the lastest netdev code.
> >
> > Of course you will want to remove all of the 'out of tree'
LINUX_VERSION
> > build macros and any other legacy bits to follow mainline kernel
> > convention for your 'in-kernel' softiwarp tree.
> >
> >
>
> And then post a patch series for review.
>
All,
Yes, ok, thats what I will do now.
Many thanks for the helpful and encouraging replies.
Bernard.
^ permalink raw reply
* Re: [PATCH 2/2 -next] r8169: use device model DMA API
From: Stanislaw Gruszka @ 2010-09-23 15:23 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: Francois Romieu, netdev
In-Reply-To: <AANLkTin-GPjkpp3gmS=1u5wxmxFMy_B6GL1MuDR79fDQ@mail.gmail.com>
On Thu, 23 Sep 2010 18:59:10 +0400
Denis Kirjanov <kirjanov@gmail.com> wrote:
> On Thu, Sep 23, 2010 at 4:01 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> > Use DMA API as PCI equivalents will be deprecated. This change also
> > allow to allocate with GFP_KERNEL where possible.
> >
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
>
> dma_map_single and friend can fail. Thus, we should check for a return value.
> Yes, this is not directly related to current patch, but it would be
> great to fix this.
Sure, I will another patch with fix soon.
Stanislaw
^ permalink raw reply
* Re: [ABI REVIEW][PATCH 0/8] Namespace file descriptors
From: David Lamparter @ 2010-09-23 15:18 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-kernel, Linux Containers, netdev, netfilter-devel,
linux-fsdevel, jamal, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Pavel Emelyanov, Ben Greear,
Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
Jan Engelhardt, Patrick McHardy
In-Reply-To: <m1ocborgq7.fsf@fess.ebiederm.org>
On Thu, Sep 23, 2010 at 01:45:04AM -0700, Eric W. Biederman wrote:
> Introduce file for manipulating namespaces and related syscalls.
> files:
> /proc/self/ns/<nstype>
As feedback from using network namespaces extensively in more or less
production setups, I would like to make a request/suggestion: there
needs to be a way to enumerate network namespaces independent from
by-pid access.
At several occasions, I was left with either some runaway daemon which
kept the namespace alive. To describe this a little more graphically:
I found no other way than doing a
md5sum /proc/*/net/if_inet6 | sort | uniq -c -w 32
to find out which runaway to kill to terminate the namespace.
This makes network namespaces particularly cumbersome to use without PID
namespaces. While I agree that a large part of the users - namely lxc -
will use them together, network namespaces without pidns are very
interesting for routing applications implementing VRFs.
Is it possible to add some kind of "all namespaces" list, optimally
giving an opportunity to open() exactly this file descriptor that you
get from /proc/<pid>/ns/net?
Also, is it possible to extend that file descriptor to have an
"get all pids" ioctl,
...or, wait, maybe have /proc/...ns/proc/<pid> symlink?
(This obviously isn't fully thought to the end, please pick up...)
-David
^ permalink raw reply
* Re: [PATCH 7/8] net: Allow setting the network namespace by fd
From: Eric W. Biederman @ 2010-09-23 15:14 UTC (permalink / raw)
To: hadi
Cc: linux-kernel, Linux Containers, netdev, netfilter-devel,
linux-fsdevel, Daniel Lezcano, Linus Torvalds, Michael Kerrisk,
Ulrich Drepper, Al Viro, David Miller, Serge E. Hallyn,
Pavel Emelyanov, Pavel Emelyanov, Ben Greear, Matt Helsley,
Jonathan Corbet, Sukadev Bhattiprolu, Jan Engelhardt,
Patrick McHardy
In-Reply-To: <1285240926.5036.7.camel@bigi>
jamal <hadi@cyberus.ca> writes:
> On Thu, 2010-09-23 at 01:51 -0700, Eric W. Biederman wrote:
>> Take advantage of the new abstraction and allow network devices
>> to be placed in any network namespace that we have a fd to talk
>> about.
>>
>
> So ... why just netdevice? could you allow migration of other
> net "items" eg a route table since they are all tagged by
> netns?
For this patchset because we only support migrating physical
network devices between network namespaces today.
In the bigger picture migrating things between network namespaces is
race prone. Fixing those races probably would reduce network stack
performance and increase code complexity for not particularly good
reason. Network devices are special because they are physical hardware
and in combination with the rule that all packets coming a network
device go to a single network namespace we have to implement migration
for network devices.
Eric
^ permalink raw reply
* RE: [PATCH] Use firmware provided index to register a network interface
From: Narendra_K @ 2010-09-23 15:13 UTC (permalink / raw)
To: tim, shemminger
Cc: netdev, linux-hotplug, linux-pci, Matt_Domsch, Charles_Rose,
Jordan_Hargrave, Vijay_Nijhawan
In-Reply-To: <4C9AF4EC.7020408@seoss.co.uk>
> -----Original Message-----
> From: Tim Small [mailto:tim@seoss.co.uk]
> Sent: Thursday, September 23, 2010 12:04 PM
> To: Stephen Hemminger
> Cc: K, Narendra; netdev@vger.kernel.org;
linux-hotplug@vger.kernel.org;
> linux-pci@vger.kernel.org; Domsch, Matt; Rose, Charles; Hargrave,
> Jordan; Nijhawan, Vijay
> Subject: Re: [PATCH] Use firmware provided index to register a network
> interface
>
> Stephen Hemminger wrote:
> >>> http://marc.info/?l=linux-netdev&m=125510301513312&w=2
> >> Out of interest, that link says that doing it in usespace was
> rejected,
> >> but doesn't give any references... I'd be interested to know why
> this
> >> wasn't viable - since this seemed like the best fit at first glance
> -
> >> most people will never use this, so no need to grow their kernel
> size
> >> and complexity?
> >>
> >>
> >
> > This proposal was to ad changes into every application that
> > knows about network names (iproute, iptables, snmp, quagga,
openswan,
> ...)
> > to do aliasing at the application layer.
> >
>
>
> OK, that's bonkers, but what I was referring to was the line in the
> linked post which said "Achieve the above in userspace only using
udev"
> - which I assumed meant to do it once in a udev rename rule by
adapting
> /etc/udev/rules.d/70-persistent-net.rules , /lib/udev/write_net_rules
> etc. - which is what I've used to enforce this sort of convention
> myself
> from time to time.
>
Hi,
I was referring to the solution proposed in this thread -
http://marc.info/?l=linux-netdev&m=125619338904322&w=3 ([PATCH] udev:
create empty regular files to represent net).
With regards,
Narendra K
^ permalink raw reply
* RE: [PATCH] Use firmware provided index to register a network interface
From: Narendra_K @ 2010-09-23 15:10 UTC (permalink / raw)
To: greg
Cc: netdev, linux-hotplug, linux-pci, Matt_Domsch, Charles_Rose,
Jordan_Hargrave, Vijay_Nijhawan
In-Reply-To: <20100922192228.GB29899@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, September 23, 2010 12:52 AM
> To: K, Narendra
> Cc: netdev@vger.kernel.org; linux-hotplug@vger.kernel.org; linux-
> pci@vger.kernel.org; Domsch, Matt; Rose, Charles; Hargrave, Jordan;
> Nijhawan, Vijay
> Subject: Re: [PATCH] Use firmware provided index to register a network
> interface
>
> On Wed, Sep 22, 2010 at 01:31:38PM -0500, Narendra K wrote:
> > Hello,
> >
> > Here is another approach to address the issue of "eth0 does not
> always
> > map to the Integrated NIC Port 1 as denoted on server chassis
label".
> > For more details please refer to the thread -
> > http://marc.info/?l=linux-netdev&m=128163454631618&w=3.
> >
> > Patch makes use of the firmware provided index to derive ethN names.
> > That way the naming scheme adheres to the existing requirements of
> > ethN namespace and with IFNAMSIZ length.
>
> Ick, again, what's wrong with using udev for this as it is designed
to?
> That way no kernel changes are needed, and no one has to rely on the
> BIOS getting the firmware number right (meaning it will work on all
> types of systems.)
>
1. We tried addressing this issue using udev only and without any kernel
changes, during Nov-Dec 2008 timeframe using Biosdevname udev helper
utility. Biosdevname utility has the ability to suggest BIOS intended
name of an interface given its OS name.
/sbin/biosdevname -I eth2 - (OS name)
eth0 - (Name according to the BIOS)
KERNEL!="eth*", GOTO="biosdevname_end"
ACTION!="add", GOTO="biosdevname_end"
NAME=="?*", GOTO="biosdevname_end"
PROGRAM="/sbin/biosdevname --policy=all_ethN -i %k",
ENV{INTERFACE_NAME}="%c"
LABEL="biosdevname_end"
We observed that renames in the same namespace, which is ethN namespace,
resulted in interface names like eth_rename_ren. The solution was
susceptible to driver load order. Please refer to this bug report-
https://bugzilla.novell.com/show_bug.cgi?id=441079.
This solution was not favored.
2. Based on the suggestions, I am trying to see if we can make use of
the attribute 'index' that is available to udev as a result of the patch
-
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h
=911e1c9b05a8e3559a7aa89083930700a0b9e7ee (PCI: export SMBIOS provided
firmware instance and label to sysfs).(I am not using any udev helper in
this experiment).
On a PowerEdge R805, with 4 BCM5708 Lan-On-Motherboard ports and a dual
port Intel 82576 NIC. In this system the 82576 ports become eth0 and
eth1 and the Lan-On-Motherboard ports are eth2-eth5. Below is how the
70-persistent-net.rules file looks before I modified it -
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1e:4f:fc:1b:32", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth3"
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1e:4f:fc:1b:30", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth2"
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1e:4f:fc:00:40", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth5"
# PCI device 0x8086:0x10c9 (igb) (custom name provided by external tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1b:21:54:33:3c", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth0"
# PCI device 0x8086:0x10c9 (igb) (custom name provided by external tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1b:21:54:33:3d", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth1"
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1e:4f:fc:00:3e", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth4"
I modified the rules to use the ATTR{index} as below to get expected
names -
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{index}=="1",
ATTR{type}=="1", KERNEL=="eth*", NAME="eth0"
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{index}=="2",
ATTR{type}=="1", KERNEL=="eth*", NAME="eth1"
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{index}=="3",
ATTR{type}=="1", KERNEL=="eth*", NAME="eth2"
# PCI device 0x8086:0x10c9 (igb) (custom name provided by external tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1b:21:54:33:3c", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth4"
# PCI device 0x8086:0x10c9 (igb) (custom name provided by external tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*",
ATTR{address}=="00:1b:21:54:33:3d", ATTR{type}=="1", KERNEL=="eth*",
NAME="eth5"
# PCI device 0x14e4:0x164c (bnx2) (custom name provided by external
tool)
SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{index}=="4",
ATTR{type}=="1", KERNEL=="eth*", NAME="eth3"
I observe that the Lan-On-Motherboard ports get named beyond the add-in
interfaces. That is, in the original file the add-in interfaces were
eth4 and eth5. They remain same as expected. But Lan-On-Motherboard
ports get eth6,eth7,eth8,eth9. Dmesg shows the renames that are done
igb 0000:21:00.1: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
udev: renamed network interface eth1 to eth5
udev: renamed network interface eth0 to eth4
bnx2 0000:0a:00.0: eth3: Broadcom NetXtreme II BCM5708 1000Base-T (B2)
PCI-X 64-bit 133MHz found at mem ec000000, IRQ 19, node addr
00:1e:4f:fc:00:40
udev: renamed network interface eth0 to eth6
udev: renamed network interface eth2 to eth7
udev: renamed network interface eth3 to eth8
udev: renamed network interface eth1 to eth9
I am trying to figure out what might be going on.
With regards,
Narendra K
^ permalink raw reply
* [PATCH net-next-2.6] net: skb_frag_t can be smaller on small arches
From: Eric Dumazet @ 2010-09-23 15:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
David
I remember you reacted on my three years old comment about
commit a309bb072b96bfe (Page offsets and lengths need to be __u32.)
http://www.mail-archive.com/netdev@vger.kernel.org/msg43999.html
Could we reconsider the thing now and allow to shrink
sizeof(skb_shared_info) from 0x104 to 0xbc, considering nothing happened
about scatterlist work ?
This saves 128 bytes on i386 because of alignments, and can be reverted
pretty fast if needed.
Thanks !
[PATCH net-next-2.6] net: skb_frag_t can be smaller on small arches
On 32bit arches, if PAGE_SIZE is smaller than 65536, we can use 16bit
offset and size fields. This patch saves 72 bytes per skb on i386, or
128 bytes after rounding.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/skbuff.h | 5 +++++
1 files changed, 5 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9e8085a..9a7ea70 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -129,8 +129,13 @@ typedef struct skb_frag_struct skb_frag_t;
struct skb_frag_struct {
struct page *page;
+#if (BITS_PER_LONG > 32) || (PAGE_SIZE >= 65536)
__u32 page_offset;
__u32 size;
+#else
+ __u16 page_offset;
+ __u16 size;
+#endif
};
#define HAVE_HW_TIME_STAMP
^ permalink raw reply related
* Re: [RFC PATCH] dont create cached routes from ARP requests
From: Eric Dumazet @ 2010-09-23 15:05 UTC (permalink / raw)
To: Ulrich Weber; +Cc: David Miller, netdev
In-Reply-To: <20100923144708.GA8037@babylon>
Le jeudi 23 septembre 2010 à 16:47 +0200, Ulrich Weber a écrit :
> On Wed, Sep 22, 2010 at 08:34:42PM -0700, David Miller wrote:
> > From: Ulrich Weber <uweber@astaro.com>
> > Date: Wed, 22 Sep 2010 18:22:09 +0200
> >
> > > Background: At home I have two Internet connections, DSL and Cable.
> > > DSL is the primary uplink while Cable is the secondary.
> > > My Cable ISP is flooding me with ARP request from 10.0.0.0/8,
> > > which creates routes via the primary uplink.
> > > There are thousands of cached routes and after some time
> > > I get "Neighbour table overflow" messages.
> >
> > If you get neighbour table overflows, something is holding a reference
> > to the routing cache entry and/or the neighbour entries those routing
> > cache entries are attached to.
> >
> > If these really are transient entries, they should be trivially
> > garbage collected and not cause any problems at all.
> rt_garbage_collect is not called within rt_intern_hash,
> because the call is done within softirq context.
>
> Forcing the call of rt_garbage_collect didn't help either,
> there are no routes freed afterwards...
>
Please give us more information.
grep . /proc/sys/net/ipv4/route/*
rtstat -c10 -i1
Thanks
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: Eric W. Biederman @ 2010-09-23 15:00 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: hadi, linux-kernel, Linux Containers, netdev, netfilter-devel,
linux-fsdevel, Daniel Lezcano, Linus Torvalds, Michael Kerrisk,
Ulrich Drepper, Al Viro, David Miller, Serge E. Hallyn,
Ben Greear, Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
Jan Engelhardt, Patrick McHardy
In-Reply-To: <4C9B495D.70200@parallels.com>
Pavel Emelyanov <xemul@parallels.com> writes:
> On 09/23/2010 04:11 PM, jamal wrote:
>> On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
>>
>>> Why does it matter? You told, that the usage scenario was to
>>> add routes to container. If I do 2 syscalls instead of 1, is
>>> it THAT worse?
>>>
>>
>> Anything to do with socket IO that requires namespace awareness
>> applies for usage; it could be tcp/udp/etc socket. If it doesnt
>> make any difference performance wise using one scheme vs other
>> to write/read heavy messages then i dont see an issue and socketat
>> is redundant.
>
> That's what my point is about - unless we know why would we need it
> we don't need it.
>
> Eric, please clarify, what is the need in creating a socket in foreign
> net namespace?
Strictly speaking with setns() you can implement this functionality
with setns(). aka
int socketat(int nsfd, int domain, int type, int protocol)
{
int sk;
setns(0, nsfd);
sk = socket(domain, type, protocol);
setns(0, default_nsfd);
return sk;
}
The major difference is that socketat in userspace suffers
from races, with signals etc.
The use case are applications are the handful of networking applications
that find that it makes sense to listen to sockets from multiple network
namespaces at once. Say a home machine that has a vpn into your office
network and the vpn into the office network runs in a different network
namespace so you don't have to worry about address conflicts between
the two networks, the chance of accidentally bridging between them,
and so you can use different dns resolvers for the different networks.
In that scenario it would be nice if I could run some services on both
networks. Starting two+ copies of the daemons just so the can have live
in all of the networks is ok, but in the fullness of time I expect that
there will be daemons that want to optimize things and have sockets in
all of the network namespaces you are connected to.
In a multiple network namespace aware application when it goes to open
a socket it will want to specify which network namespace the socket is
in. If it is a general listener it will probably listening to events
in /proc/mounts waiting for extra namespaces to be mounted under a
standard location say: /var/run/netns/<netnsname>/ns.
Once the application receives the event for a new network namespace
showing up it can will want to create a new socket listening for
connections in the new network namespace.
In that scenario none of those network namespaces are foreign, but one
network namespace will be the default and the rest will be non-default
network namespaces.
To support a multiple network namespace aware daemon I need to implement
sockeat() somewhere. So I figured I would see if anyone minded a
trivial in kernel race free implementation. To me it is a wart in the
API and I am busily removing warts in the API.
I don't know of any scenarios with other namespaces where there would be
applications that would be native in multiple namespaces. So I haven't
haven't done any work in that direction.
Eric
^ permalink raw reply
* Re: [PATCH 2/2 -next] r8169: use device model DMA API
From: Denis Kirjanov @ 2010-09-23 14:59 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Francois Romieu, netdev
In-Reply-To: <1285243291-4520-2-git-send-email-sgruszka@redhat.com>
On Thu, Sep 23, 2010 at 4:01 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Use DMA API as PCI equivalents will be deprecated. This change also
> allow to allocate with GFP_KERNEL where possible.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
dma_map_single and friend can fail. Thus, we should check for a return value.
Yes, this is not directly related to current patch, but it would be
great to fix this.
> drivers/net/r8169.c | 53 +++++++++++++++++++++++++++-----------------------
> 1 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 4f94073..51dd9ac 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -1217,7 +1217,8 @@ static void rtl8169_update_counters(struct net_device *dev)
> if ((RTL_R8(ChipCmd) & CmdRxEnb) == 0)
> return;
>
> - counters = pci_alloc_consistent(tp->pci_dev, sizeof(*counters), &paddr);
> + counters = dma_alloc_coherent(&tp->pci_dev->dev, sizeof(*counters),
> + &paddr, GFP_KERNEL);
> if (!counters)
> return;
>
> @@ -1238,7 +1239,8 @@ static void rtl8169_update_counters(struct net_device *dev)
> RTL_W32(CounterAddrLow, 0);
> RTL_W32(CounterAddrHigh, 0);
>
> - pci_free_consistent(tp->pci_dev, sizeof(*counters), counters, paddr);
> + dma_free_coherent(&tp->pci_dev->dev, sizeof(*counters), counters,
> + paddr);
> }
>
> static void rtl8169_get_ethtool_stats(struct net_device *dev,
> @@ -3298,15 +3300,15 @@ static int rtl8169_open(struct net_device *dev)
>
> /*
> * Rx and Tx desscriptors needs 256 bytes alignment.
> - * pci_alloc_consistent provides more.
> + * dma_alloc_coherent provides more.
> */
> - tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
> - &tp->TxPhyAddr);
> + tp->TxDescArray = dma_alloc_coherent(&pdev->dev, R8169_TX_RING_BYTES,
> + &tp->TxPhyAddr, GFP_KERNEL);
> if (!tp->TxDescArray)
> goto err_pm_runtime_put;
>
> - tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
> - &tp->RxPhyAddr);
> + tp->RxDescArray = dma_alloc_coherent(&pdev->dev, R8169_RX_RING_BYTES,
> + &tp->RxPhyAddr, GFP_KERNEL);
> if (!tp->RxDescArray)
> goto err_free_tx_0;
>
> @@ -3340,12 +3342,12 @@ out:
> err_release_ring_2:
> rtl8169_rx_clear(tp);
> err_free_rx_1:
> - pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
> - tp->RxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
> + tp->RxPhyAddr);
> tp->RxDescArray = NULL;
> err_free_tx_0:
> - pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
> - tp->TxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
> + tp->TxPhyAddr);
> tp->TxDescArray = NULL;
> err_pm_runtime_put:
> pm_runtime_put_noidle(&pdev->dev);
> @@ -3981,7 +3983,7 @@ static void rtl8169_free_rx_skb(struct rtl8169_private *tp,
> {
> struct pci_dev *pdev = tp->pci_dev;
>
> - pci_unmap_single(pdev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
> + dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
> PCI_DMA_FROMDEVICE);
> dev_kfree_skb(*sk_buff);
> *sk_buff = NULL;
> @@ -4020,7 +4022,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
>
> skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
>
> - mapping = pci_map_single(pdev, skb->data, rx_buf_sz,
> + mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
> PCI_DMA_FROMDEVICE);
>
> rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
> @@ -4105,7 +4107,8 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
> {
> unsigned int len = tx_skb->len;
>
> - pci_unmap_single(pdev, le64_to_cpu(desc->addr), len, PCI_DMA_TODEVICE);
> + dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), len,
> + PCI_DMA_TODEVICE);
> desc->opts1 = 0x00;
> desc->opts2 = 0x00;
> desc->addr = 0x00;
> @@ -4249,7 +4252,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
> txd = tp->TxDescArray + entry;
> len = frag->size;
> addr = ((void *) page_address(frag->page)) + frag->page_offset;
> - mapping = pci_map_single(tp->pci_dev, addr, len, PCI_DMA_TODEVICE);
> + mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
> + PCI_DMA_TODEVICE);
>
> /* anti gcc 2.95.3 bugware (sic) */
> status = opts1 | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
> @@ -4319,7 +4323,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
> tp->tx_skb[entry].skb = skb;
> }
>
> - mapping = pci_map_single(tp->pci_dev, skb->data, len, PCI_DMA_TODEVICE);
> + mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
> + PCI_DMA_TODEVICE);
>
> tp->tx_skb[entry].len = len;
> txd->addr = cpu_to_le64(mapping);
> @@ -4482,8 +4487,8 @@ static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
> if (!skb)
> goto out;
>
> - pci_dma_sync_single_for_cpu(tp->pci_dev, addr, pkt_size,
> - PCI_DMA_FROMDEVICE);
> + dma_sync_single_for_cpu(&tp->pci_dev->dev, addr, pkt_size,
> + PCI_DMA_FROMDEVICE);
> skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
> *sk_buff = skb;
> done = true;
> @@ -4552,11 +4557,11 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
> }
>
> if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
> - pci_dma_sync_single_for_device(pdev, addr,
> + dma_sync_single_for_device(&pdev->dev, addr,
> pkt_size, PCI_DMA_FROMDEVICE);
> rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
> } else {
> - pci_unmap_single(pdev, addr, tp->rx_buf_sz,
> + dma_unmap_single(&pdev->dev, addr, tp->rx_buf_sz,
> PCI_DMA_FROMDEVICE);
> tp->Rx_skbuff[entry] = NULL;
> }
> @@ -4774,10 +4779,10 @@ static int rtl8169_close(struct net_device *dev)
>
> free_irq(dev->irq, dev);
>
> - pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
> - tp->RxPhyAddr);
> - pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
> - tp->TxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
> + tp->RxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
> + tp->TxPhyAddr);
> tp->TxDescArray = NULL;
> tp->RxDescArray = NULL;
>
> --
> 1.7.0.1
>
> --
> 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
>
--
Regards,
Denis
^ permalink raw reply
* Re: [PATCH 7/8] net: Allow setting the network namespace by fd
From: David Lamparter @ 2010-09-23 14:58 UTC (permalink / raw)
To: jamal
Cc: Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Pavel Emelyanov, Ben Greear,
Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
Jan Engelhardt, Patrick McHardy
In-Reply-To: <1285240926.5036.7.camel@bigi>
On Thu, Sep 23, 2010 at 07:22:06AM -0400, jamal wrote:
> On Thu, 2010-09-23 at 01:51 -0700, Eric W. Biederman wrote:
> > Take advantage of the new abstraction and allow network devices
> > to be placed in any network namespace that we have a fd to talk
> > about.
>
> So ... why just netdevice? could you allow migration of other
> net "items" eg a route table since they are all tagged by
> netns?
migrating route table entries makes no sense because
a) they refer to devices and configuration that does not exist in the
target namespace; they only make sense within their netns context
b) they are purely virtual and you get the same result from deleting and
recreating them.
Network devices are special because they may have something attached to
them, be it hardware or some daemon.
-David
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: David Lamparter @ 2010-09-23 14:54 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: hadi, Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Ben Greear, Matt Helsley, Jonathan Corbet,
Sukadev Bhattiprolu, Jan Engelhardt, Patrick McHardy
In-Reply-To: <4C9B495D.70200@parallels.com>
On Thu, Sep 23, 2010 at 04:34:37PM +0400, Pavel Emelyanov wrote:
> On 09/23/2010 04:11 PM, jamal wrote:
> > On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
> >
> >> Why does it matter? You told, that the usage scenario was to
> >> add routes to container. If I do 2 syscalls instead of 1, is
> >> it THAT worse?
> >>
> >
> > Anything to do with socket IO that requires namespace awareness
> > applies for usage; it could be tcp/udp/etc socket. If it doesnt
> > make any difference performance wise using one scheme vs other
> > to write/read heavy messages then i dont see an issue and socketat
> > is redundant.
>
> That's what my point is about - unless we know why would we need it
> we don't need it.
>
> Eric, please clarify, what is the need in creating a socket in foreign
> net namespace?
Hmm. If you somewhere get the fd to a socket from another namespace, it
definitely does work (I'm currently implementing my "socketat" with fd
passing through AF_UNIX sockets, so i know it works), so the
setns(other...)
fd = socket(...)
setns(orig...)
sequence would certainly work. However, there might be other things
happening inbetween like a signal (imagine AIO particularly). While
signals are user-controllable (and therefore to be managed/excluded by
the user), we need to think if there are other problems with doing this
as sequence?
If there are no other problematic conditions with this, socketat should
probably be moved to a user library.
-David
^ permalink raw reply
* Re: [RFC PATCH] dont create cached routes from ARP requests
From: Ulrich Weber @ 2010-09-23 14:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100922.203442.233700254.davem@davemloft.net>
On Wed, Sep 22, 2010 at 08:34:42PM -0700, David Miller wrote:
> From: Ulrich Weber <uweber@astaro.com>
> Date: Wed, 22 Sep 2010 18:22:09 +0200
>
> > Background: At home I have two Internet connections, DSL and Cable.
> > DSL is the primary uplink while Cable is the secondary.
> > My Cable ISP is flooding me with ARP request from 10.0.0.0/8,
> > which creates routes via the primary uplink.
> > There are thousands of cached routes and after some time
> > I get "Neighbour table overflow" messages.
>
> If you get neighbour table overflows, something is holding a reference
> to the routing cache entry and/or the neighbour entries those routing
> cache entries are attached to.
>
> If these really are transient entries, they should be trivially
> garbage collected and not cause any problems at all.
rt_garbage_collect is not called within rt_intern_hash,
because the call is done within softirq context.
Forcing the call of rt_garbage_collect didn't help either,
there are no routes freed afterwards...
Cheers
Ulrich
^ permalink raw reply
* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Eric Dumazet @ 2010-09-23 14:41 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Oliver Hartkopp, Michael S. Tsirkin, netdev
In-Reply-To: <AANLkTik4d76GBa9Jwn9BOnC07_0CnMsy-XoKM_MhF_sc@mail.gmail.com>
Le jeudi 23 septembre 2010 à 22:17 +0800, Changli Gao a écrit :
> On Thu, Sep 23, 2010 at 8:29 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le jeudi 23 septembre 2010 à 18:15 +0800, Changli Gao a écrit :
> >> Since skb->destructor() is used to account socket memory, and maybe called
> >> before the skb is sent out, a corrupt skb maybe sent out finally.
> >>
> >> A new destructor is added into structure skb_shared_info(), and it won't
> >> be called until the last reference to the data of an skb is put. af_packet
> >> uses this destructor instead.
> >>
> >> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> >> ---
> >> v3: rename destructor to data_destructor, destructor_arg to data_destructor_arg,
> >> fix splice the skbs generated by AF_PACKET socket to the pipe.
> >
> > I dont understand this.
> >
> > Could you describe how splice(from af_packet to pipe) is possible with
> > af_packet send path ?
>
> af_packet sends packets to lo(127.0.0.1), and a local socket is
> receiving packets via splice.
Ouch
I am pretty sure too many things will break if we allow such packets to
get back in input path.
(think of tcp coalescing for example...)
^ permalink raw reply
* Re: [PATCH 7/8] net: Allow setting the network namespace by fd
From: Brian Haley @ 2010-09-23 14:22 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-kernel, Linux Containers, netdev, netfilter-devel,
linux-fsdevel, jamal, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Pavel Emelyanov, Ben Greear,
Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
Jan Engelhardt, Patrick McHardy
In-Reply-To: <m1hbhgq1v1.fsf@fess.ebiederm.org>
On 09/23/2010 04:51 AM, Eric W. Biederman wrote:
>
> Take advantage of the new abstraction and allow network devices
> to be placed in any network namespace that we have a fd to talk
> about.
>
...
> +struct net *get_net_ns_by_fd(int fd)
> +{
> + struct proc_inode *ei;
> + struct file *file;
> + struct net *net;
> +
> + file = NULL;
No need to initialize this.
> + net = ERR_PTR(-EINVAL);
or this?
> + file = proc_ns_fget(fd);
> + if (!fd)
> + goto out;
> + return ERR_PTR(-EINVAL);
Shouldn't this be:
if (!file)
And the "goto" seems wrong, especially without a {} here. Unless you
meant to keep the "goto" and branch below?
-Brian
> +
> + ei = PROC_I(file->f_dentry->d_inode);
> + if (ei->ns_ops != &netns_operations)
> + goto out;
> +
> + net = get_net(ei->ns);
> +out:
> + if (file)
> + fput(file);
> + return net;
> +}
^ permalink raw reply
* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Changli Gao @ 2010-09-23 14:17 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Oliver Hartkopp, Michael S. Tsirkin, netdev
In-Reply-To: <1285244970.2864.46.camel@edumazet-laptop>
On Thu, Sep 23, 2010 at 8:29 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 23 septembre 2010 à 18:15 +0800, Changli Gao a écrit :
>> Since skb->destructor() is used to account socket memory, and maybe called
>> before the skb is sent out, a corrupt skb maybe sent out finally.
>>
>> A new destructor is added into structure skb_shared_info(), and it won't
>> be called until the last reference to the data of an skb is put. af_packet
>> uses this destructor instead.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> v3: rename destructor to data_destructor, destructor_arg to data_destructor_arg,
>> fix splice the skbs generated by AF_PACKET socket to the pipe.
>
> I dont understand this.
>
> Could you describe how splice(from af_packet to pipe) is possible with
> af_packet send path ?
af_packet sends packets to lo(127.0.0.1), and a local socket is
receiving packets via splice.
>
> Also, on such risky patch, could you please avoid inserting cleanups ?
> I am referring to these bits :
>
OK. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH v4 2/2] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-09-23 14:16 UTC (permalink / raw)
To: Ville Tervo
Cc: Jiri Kosina, Stefan Achatz, Antonio Ospite, Alexey Dobriyan,
Tejun Heo, Alan Stern, Greg Kroah-Hartman, Marcel Holtmann,
Stephane Chatty, Michael Poole, David S. Miller, Bastien Nocera,
Eric Dumazet, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20100923115108.GC2379@null>
On Sep 23, 2010, at 7:51 AM, Ville Tervo wrote:
> Hi Alan,
>
> One comment.
>
> How about a variable called ret and using that to return len or
> errno? It
> would eliminate code dublication.
>
Hi Ville,
Where specifically? In which function? I've gone through it a couple
of times and failed to find return statements which are superfluous.
Maybe I'm missing something fundamental?
Alan.
^ permalink raw reply
* RE: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
From: Xin, Xiaohui @ 2010-09-23 12:56 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mingo@elte.hu, davem@davemloft.net,
herbert@gondor.hengli.com.au, jdike@linux.intel.com
In-Reply-To: <20100922115505.GF16423@redhat.com>
>-----Original Message-----
>From: Michael S. Tsirkin [mailto:mst@redhat.com]
>Sent: Wednesday, September 22, 2010 7:55 PM
>To: Xin, Xiaohui
>Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>jdike@linux.intel.com
>Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
>
>On Wed, Sep 22, 2010 at 07:41:36PM +0800, Xin, Xiaohui wrote:
>> >-----Original Message-----
>> >From: Michael S. Tsirkin [mailto:mst@redhat.com]
>> >Sent: Tuesday, September 21, 2010 9:14 PM
>> >To: Xin, Xiaohui
>> >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>> >mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>> >jdike@linux.intel.com
>> >Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
>> >
>> >On Tue, Sep 21, 2010 at 09:39:31AM +0800, Xin, Xiaohui wrote:
>> >> >From: Michael S. Tsirkin [mailto:mst@redhat.com]
>> >> >Sent: Monday, September 20, 2010 7:37 PM
>> >> >To: Xin, Xiaohui
>> >> >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>> >> >mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>> >> >jdike@linux.intel.com
>> >> >Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
>> >> >
>> >> >On Mon, Sep 20, 2010 at 04:08:48PM +0800, xiaohui.xin@intel.com wrote:
>> >> >> From: Xin Xiaohui <xiaohui.xin@intel.com>
>> >> >>
>> >> >> ---
>> >> >> Michael,
>> >> >> I have move the ioctl to configure the locked memory to vhost
>> >> >
>> >> >It's ok to move this to vhost but vhost does not
>> >> >know how much memory is needed by the backend.
>> >>
>> >> I think the backend here you mean is mp device.
>> >> Actually, the memory needed is related to vq->num to run zero-copy
>> >> smoothly.
>> >> That means mp device did not know it but vhost did.
>> >
>> >Well, this might be so if you insist on locking
>> >all posted buffers immediately. However, let's assume I have a
>> >very large ring and prepost a ton of RX buffers:
>> >there's no need to lock all of them directly:
>> >
>> >if we have buffers A and B, we can lock A, pass it
>> >to hardware, and when A is consumed unlock A, lock B
>> >and pass it to hardware.
>> >
>> >
>> >It's not really critical. But note we can always have userspace
>> >tell MP device all it wants to know, after all.
>> >
>> Ok. Here are two values we have mentioned, one is how much memory
>> user application wants to lock, and one is how much memory locked
>> is needed to run smoothly. When net backend is setup, we first need
>> an ioctl to get how much memory is needed to lock, and then we call
>> another ioctl to set how much it want to lock. Is that what's in your mind?
>
>That's fine.
>
>> >> And the rlimt stuff is per process, we use current pointer to set
>> >> and check the rlimit, the operations should be in the same process.
>> >
>> >Well no, the ring is handled from the kernel thread: we switch the mm to
>> >point to the owner task so copy from/to user and friends work, but you
>> >can't access the rlimit etc.
>> >
>> Yes, the userspace and vhost kernel is not the same process. But we can
>> record the task pointer as mm.
>
>So you will have to store mm and do device->mm, not current->mm.
>Anyway, better not touch mm on data path.
>
>> >> Now the check operations are in vhost process, as mp_recvmsg() or
>> >> mp_sendmsg() are called by vhost.
>> >
>> >Hmm, what do you mean by the check operations?
>> >send/recv are data path operations, they shouldn't
>> >do any checks, should they?
>> >
>> As you mentioned what infiniband driver done:
>> down_write(¤t->mm->mmap_sem);
>>
>> locked = npages + current->mm->locked_vm;
>> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>>
>> if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) {
>> ret = -ENOMEM;
>> goto out;
>> }
>>
>> cur_base = addr & PAGE_MASK;
>>
>> ret = 0;
>> while (npages) {
>> ret = get_user_pages(current, current->mm, cur_base,
>> min_t(unsigned long, npages,
>> PAGE_SIZE / sizeof (struct page *)),
>> 1, !umem->writable, page_list, vma_list);
>>
>> I think it's a data path too.
>
>in infiniband this is used to 'register memory' which is not data path.
>
>> We do the check because get_user_pages() really pin and locked
>> the memory.
>
>Don't do this. Performance will be bad.
>Do the check once in ioctl and increment locked_vm by max amount you will use.
>On data path just make sure you do not exceed what userspace told you
>to.
What's in my mind is that in the ioctl which to get the memory locked needed to run smoothly,
it just return a value of how much memory is needed by mp device.
And then in the ioctl which to set the memory locked by user space, it check the rlimit and
increment locked_vm by user want. But I'm not sure how to "make sure do not exceed what
userspace told to". If we don't check locked_vm, what do we use to check? And Is it another kind of check on data path?
>
>>
>> >> So set operations should be in
>> >> vhost process too, it's natural.
>> >>
>> >> >So I think we'll need another ioctl in the backend
>> >> >to tell userspace how much memory is needed?
>> >> >
>> >> Except vhost tells it to mp device, mp did not know
>> >> how much memory is needed to run zero-copy smoothly.
>> >> Is userspace interested about the memory mp is needed?
>> >
>> >Couldn't parse this last question.
>> >I think userspace generally does want control over
>> >how much memory we'll lock. We should not just lock
>> >as much as we can.
>> >
>> >--
>> >MST
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: Pavel Emelyanov @ 2010-09-23 12:34 UTC (permalink / raw)
To: hadi-fAAogVwAN2Kw5LPnMra/2Q, Eric W. Biederman
Cc: Sukadev Bhattiprolu, Jonathan Corbet, Ulrich Drepper,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jan Engelhardt,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk,
Linux Containers, Ben Greear, Linus Torvalds, David Miller,
Al Viro
In-Reply-To: <1285243881.5036.22.camel@bigi>
On 09/23/2010 04:11 PM, jamal wrote:
> On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
>
>> Why does it matter? You told, that the usage scenario was to
>> add routes to container. If I do 2 syscalls instead of 1, is
>> it THAT worse?
>>
>
> Anything to do with socket IO that requires namespace awareness
> applies for usage; it could be tcp/udp/etc socket. If it doesnt
> make any difference performance wise using one scheme vs other
> to write/read heavy messages then i dont see an issue and socketat
> is redundant.
That's what my point is about - unless we know why would we need it
we don't need it.
Eric, please clarify, what is the need in creating a socket in foreign
net namespace?
^ permalink raw reply
* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Eric Dumazet @ 2010-09-23 12:29 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Oliver Hartkopp, Michael S. Tsirkin, netdev
In-Reply-To: <1285236939-3239-1-git-send-email-xiaosuo@gmail.com>
Le jeudi 23 septembre 2010 à 18:15 +0800, Changli Gao a écrit :
> Since skb->destructor() is used to account socket memory, and maybe called
> before the skb is sent out, a corrupt skb maybe sent out finally.
>
> A new destructor is added into structure skb_shared_info(), and it won't
> be called until the last reference to the data of an skb is put. af_packet
> uses this destructor instead.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> v3: rename destructor to data_destructor, destructor_arg to data_destructor_arg,
> fix splice the skbs generated by AF_PACKET socket to the pipe.
I dont understand this.
Could you describe how splice(from af_packet to pipe) is possible with
af_packet send path ?
Also, on such risky patch, could you please avoid inserting cleanups ?
I am referring to these bits :
@@ -884,9 +883,8 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
to_write = tp_len;
if (sock->type == SOCK_DGRAM) {
- err = dev_hard_header(skb, dev, ntohs(proto), addr,
- NULL, tp_len);
- if (unlikely(err < 0))
+ if (unlikely(dev_hard_header(skb, dev, ntohs(proto), addr,
+ NULL, tp_len) < 0))
return -EINVAL;
} else if (dev->hard_header_len) {
/* net device doesn't like empty head */
@@ -897,8 +895,7 @@ static int tpacket_fill_skb(struct packet_sock *po,
struct sk_buff *skb,
}
skb_push(skb, dev->hard_header_len);
- err = skb_store_bits(skb, 0, data,
- dev->hard_header_len);
+ err = skb_store_bits(skb, 0, data, dev->hard_header_len);
if (unlikely(err))
return err;
@@ -906,7 +903,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
to_write -= dev->hard_header_len;
}
- err = -EFAULT;
page = virt_to_page(data);
offset = offset_in_page(data);
len_max = PAGE_SIZE - offset;
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: jamal @ 2010-09-23 12:11 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Ben Greear, Matt Helsley,
Jonathan Corbet, Sukadev Bhattiprolu, Jan Engelhardt,
Patrick McHardy
In-Reply-To: <4C9B3F9C.8080506@parallels.com>
On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
> Why does it matter? You told, that the usage scenario was to
> add routes to container. If I do 2 syscalls instead of 1, is
> it THAT worse?
>
Anything to do with socket IO that requires namespace awareness
applies for usage; it could be tcp/udp/etc socket. If it doesnt
make any difference performance wise using one scheme vs other
to write/read heavy messages then i dont see an issue and socketat
is redundant.
If i was to pick blindly - I would say whatever approach with
less syscalls is better even if just a "slow" path one time
thing. I could create a scenario which would make it bad
to have more syscalls.
But theres also the simplicity aspect in doing:
fdx = socketat namespace foo
use fdx for read/write/poll into foo without any wrapper code.
Vs
enter foo
fdx = socket ..
read/write fdx
leave foo.
> Just like it used to before the enter.
>
So if i enter foo, get a fdx, leave foo i can use it in
ns0 as if it was in ns0?
cheers,
jamal
^ permalink raw reply
* [PATCH 2/2 -next] r8169: use device model DMA API
From: Stanislaw Gruszka @ 2010-09-23 12:01 UTC (permalink / raw)
To: Francois Romieu, netdev; +Cc: Stanislaw Gruszka
In-Reply-To: <1285243291-4520-1-git-send-email-sgruszka@redhat.com>
Use DMA API as PCI equivalents will be deprecated. This change also
allow to allocate with GFP_KERNEL where possible.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/r8169.c | 53 +++++++++++++++++++++++++++-----------------------
1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 4f94073..51dd9ac 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1217,7 +1217,8 @@ static void rtl8169_update_counters(struct net_device *dev)
if ((RTL_R8(ChipCmd) & CmdRxEnb) == 0)
return;
- counters = pci_alloc_consistent(tp->pci_dev, sizeof(*counters), &paddr);
+ counters = dma_alloc_coherent(&tp->pci_dev->dev, sizeof(*counters),
+ &paddr, GFP_KERNEL);
if (!counters)
return;
@@ -1238,7 +1239,8 @@ static void rtl8169_update_counters(struct net_device *dev)
RTL_W32(CounterAddrLow, 0);
RTL_W32(CounterAddrHigh, 0);
- pci_free_consistent(tp->pci_dev, sizeof(*counters), counters, paddr);
+ dma_free_coherent(&tp->pci_dev->dev, sizeof(*counters), counters,
+ paddr);
}
static void rtl8169_get_ethtool_stats(struct net_device *dev,
@@ -3298,15 +3300,15 @@ static int rtl8169_open(struct net_device *dev)
/*
* Rx and Tx desscriptors needs 256 bytes alignment.
- * pci_alloc_consistent provides more.
+ * dma_alloc_coherent provides more.
*/
- tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
- &tp->TxPhyAddr);
+ tp->TxDescArray = dma_alloc_coherent(&pdev->dev, R8169_TX_RING_BYTES,
+ &tp->TxPhyAddr, GFP_KERNEL);
if (!tp->TxDescArray)
goto err_pm_runtime_put;
- tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
- &tp->RxPhyAddr);
+ tp->RxDescArray = dma_alloc_coherent(&pdev->dev, R8169_RX_RING_BYTES,
+ &tp->RxPhyAddr, GFP_KERNEL);
if (!tp->RxDescArray)
goto err_free_tx_0;
@@ -3340,12 +3342,12 @@ out:
err_release_ring_2:
rtl8169_rx_clear(tp);
err_free_rx_1:
- pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
- tp->RxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
+ tp->RxPhyAddr);
tp->RxDescArray = NULL;
err_free_tx_0:
- pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
- tp->TxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
+ tp->TxPhyAddr);
tp->TxDescArray = NULL;
err_pm_runtime_put:
pm_runtime_put_noidle(&pdev->dev);
@@ -3981,7 +3983,7 @@ static void rtl8169_free_rx_skb(struct rtl8169_private *tp,
{
struct pci_dev *pdev = tp->pci_dev;
- pci_unmap_single(pdev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
+ dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
PCI_DMA_FROMDEVICE);
dev_kfree_skb(*sk_buff);
*sk_buff = NULL;
@@ -4020,7 +4022,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
- mapping = pci_map_single(pdev, skb->data, rx_buf_sz,
+ mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
PCI_DMA_FROMDEVICE);
rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
@@ -4105,7 +4107,8 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
{
unsigned int len = tx_skb->len;
- pci_unmap_single(pdev, le64_to_cpu(desc->addr), len, PCI_DMA_TODEVICE);
+ dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), len,
+ PCI_DMA_TODEVICE);
desc->opts1 = 0x00;
desc->opts2 = 0x00;
desc->addr = 0x00;
@@ -4249,7 +4252,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
txd = tp->TxDescArray + entry;
len = frag->size;
addr = ((void *) page_address(frag->page)) + frag->page_offset;
- mapping = pci_map_single(tp->pci_dev, addr, len, PCI_DMA_TODEVICE);
+ mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
+ PCI_DMA_TODEVICE);
/* anti gcc 2.95.3 bugware (sic) */
status = opts1 | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
@@ -4319,7 +4323,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
tp->tx_skb[entry].skb = skb;
}
- mapping = pci_map_single(tp->pci_dev, skb->data, len, PCI_DMA_TODEVICE);
+ mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
+ PCI_DMA_TODEVICE);
tp->tx_skb[entry].len = len;
txd->addr = cpu_to_le64(mapping);
@@ -4482,8 +4487,8 @@ static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
if (!skb)
goto out;
- pci_dma_sync_single_for_cpu(tp->pci_dev, addr, pkt_size,
- PCI_DMA_FROMDEVICE);
+ dma_sync_single_for_cpu(&tp->pci_dev->dev, addr, pkt_size,
+ PCI_DMA_FROMDEVICE);
skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
*sk_buff = skb;
done = true;
@@ -4552,11 +4557,11 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
}
if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
- pci_dma_sync_single_for_device(pdev, addr,
+ dma_sync_single_for_device(&pdev->dev, addr,
pkt_size, PCI_DMA_FROMDEVICE);
rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
} else {
- pci_unmap_single(pdev, addr, tp->rx_buf_sz,
+ dma_unmap_single(&pdev->dev, addr, tp->rx_buf_sz,
PCI_DMA_FROMDEVICE);
tp->Rx_skbuff[entry] = NULL;
}
@@ -4774,10 +4779,10 @@ static int rtl8169_close(struct net_device *dev)
free_irq(dev->irq, dev);
- pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
- tp->RxPhyAddr);
- pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
- tp->TxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
+ tp->RxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
+ tp->TxPhyAddr);
tp->TxDescArray = NULL;
tp->RxDescArray = NULL;
--
1.7.0.1
^ permalink raw reply related
* [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Stanislaw Gruszka @ 2010-09-23 12:01 UTC (permalink / raw)
To: Francois Romieu, netdev; +Cc: Stanislaw Gruszka
We have fedora bug report where driver fail to initialize after
suspend/resume because of memory allocation errors:
https://bugzilla.redhat.com/show_bug.cgi?id=629158
To fix use GFP_KERNEL allocation where possible.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/r8169.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 5490033..4f94073 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4006,7 +4006,7 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
struct net_device *dev,
struct RxDesc *desc, int rx_buf_sz,
- unsigned int align)
+ unsigned int align, gfp_t gfp)
{
struct sk_buff *skb;
dma_addr_t mapping;
@@ -4014,7 +4014,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
pad = align ? align : NET_IP_ALIGN;
- skb = netdev_alloc_skb(dev, rx_buf_sz + pad);
+ skb = __netdev_alloc_skb(dev, rx_buf_sz + pad, gfp);
if (!skb)
goto err_out;
@@ -4045,7 +4045,7 @@ static void rtl8169_rx_clear(struct rtl8169_private *tp)
}
static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
- u32 start, u32 end)
+ u32 start, u32 end, gfp_t gfp)
{
u32 cur;
@@ -4060,7 +4060,7 @@ static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
skb = rtl8169_alloc_rx_skb(tp->pci_dev, dev,
tp->RxDescArray + i,
- tp->rx_buf_sz, tp->align);
+ tp->rx_buf_sz, tp->align, gfp);
if (!skb)
break;
@@ -4088,7 +4088,7 @@ static int rtl8169_init_ring(struct net_device *dev)
memset(tp->tx_skb, 0x0, NUM_TX_DESC * sizeof(struct ring_info));
memset(tp->Rx_skbuff, 0x0, NUM_RX_DESC * sizeof(struct sk_buff *));
- if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC) != NUM_RX_DESC)
+ if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC, GFP_KERNEL) != NUM_RX_DESC)
goto err_out;
rtl8169_mark_as_last_descriptor(tp->RxDescArray + NUM_RX_DESC - 1);
@@ -4587,7 +4587,7 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
count = cur_rx - tp->cur_rx;
tp->cur_rx = cur_rx;
- delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx);
+ delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx, GFP_ATOMIC);
if (!delta && count)
netif_info(tp, intr, dev, "no Rx buffer allocated\n");
tp->dirty_rx += delta;
--
1.7.0.1
^ permalink raw reply related
* Re: [PATCH 8/8] net: Implement socketat.
From: Pavel Emelyanov @ 2010-09-23 11:53 UTC (permalink / raw)
To: hadi
Cc: Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Ben Greear, Matt Helsley,
Jonathan Corbet, Sukadev Bhattiprolu, Jan Engelhardt,
Patrick McHardy
In-Reply-To: <1285242055.5036.9.camel@bigi>
On 09/23/2010 03:40 PM, jamal wrote:
> On Thu, 2010-09-23 at 15:33 +0400, Pavel Emelyanov wrote:
>
>> This particular usecase is unneeded once you have the "enter" ability.
>
> Is that cheaper from a syscall count/cost?
Why does it matter? You told, that the usage scenario was to
add routes to container. If I do 2 syscalls instead of 1, is
it THAT worse?
> i.e do I have to enter every time i want to write/read this fd?
No - you enter once, create a socket and do whatever you need
withing the enterned namespace.
> How does poll/select work in that enter scenario?
Just like it used to before the enter.
> cheers,
> jamal
>
>
^ 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