* Re: Unaligned access in xfrm_user:copy_to_user_state
From: David Miller @ 2010-04-02 21:03 UTC (permalink / raw)
To: jengelh; +Cc: netdev
In-Reply-To: <alpine.LSU.2.01.1004022210270.30875@obet.zrqbmnf.qr>
From: Jan Engelhardt <jengelh@medozas.de>
Date: Fri, 2 Apr 2010 22:18:59 +0200 (CEST)
> since we seem to be dealing with unaligned access quite recently, here's
> my turn in reporting one:
>
> 22:09 ares:/etc # uname -a
> Linux ares 2.6.34-rc1 #17 SMP Thu Mar 25 00:08:55 CET 2010 sparc64
> sparc64 sparc64 GNU/Linux
> (This is kaber/nf-next)
>
> Apr 2 22:09:53 ares kernel: Kernel unaligned access at TPC[101a0c18]
> copy_to_user_state+0x18/0x120 [xfrm_user]
>
> 0000000000000c00 <copy_to_user_state>:
> c00: 9d e3 bf 50 save %sp, -176, %sp
> c04: ce 5e 20 80 ldx [ %i0 + 0x80 ], %g7
> c08: 86 06 20 80 add %i0, 0x80, %g3
> c0c: 84 06 60 38 add %i1, 0x38, %g2
> c10: 82 06 20 98 add %i0, 0x98, %g1
> c14: 90 06 60 60 add %i1, 0x60, %o0
> c18: ce 76 60 38 stx %g7, [ %i1 + 0x38 ]
>
> That happens when strongswan is trying to handle a new incoming tunnel
> request between two IPv6 endpoints (it does not seem to get triggered
> for IPv4).
Yes, we need to "void *" untype the arguments to memcpy so that
GCC doesn't inline the thing.
Patches welcome.
^ permalink raw reply
* Re: [BUG] latest net-next-2.6 doesnt fly
From: David Miller @ 2010-04-02 20:35 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, fujita.tomonori
In-Reply-To: <1270202304.1989.14.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 02 Apr 2010 11:58:24 +0200
> [PATCH net-next-2.6] net: illegal_highdma() fix
>
> Followup to commit 5acbbd428db47b12f137a8a2aa96b3c0a96b744e
> (net: change illegal_highdma to use dma_mask)
>
> If dev->dev.parent is NULL, we should not try to dereference it.
>
> Dont force inline illegal_highdma() as its pretty big now.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks for tracking this down.
^ permalink raw reply
* Unaligned access in xfrm_user:copy_to_user_state
From: Jan Engelhardt @ 2010-04-02 20:18 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
Hi,
since we seem to be dealing with unaligned access quite recently, here's
my turn in reporting one:
22:09 ares:/etc # uname -a
Linux ares 2.6.34-rc1 #17 SMP Thu Mar 25 00:08:55 CET 2010 sparc64
sparc64 sparc64 GNU/Linux
(This is kaber/nf-next)
Apr 2 22:09:53 ares kernel: Kernel unaligned access at TPC[101a0c18]
copy_to_user_state+0x18/0x120 [xfrm_user]
0000000000000c00 <copy_to_user_state>:
c00: 9d e3 bf 50 save %sp, -176, %sp
c04: ce 5e 20 80 ldx [ %i0 + 0x80 ], %g7
c08: 86 06 20 80 add %i0, 0x80, %g3
c0c: 84 06 60 38 add %i1, 0x38, %g2
c10: 82 06 20 98 add %i0, 0x98, %g1
c14: 90 06 60 60 add %i1, 0x60, %o0
c18: ce 76 60 38 stx %g7, [ %i1 + 0x38 ]
That happens when strongswan is trying to handle a new incoming tunnel
request between two IPv6 endpoints (it does not seem to get triggered
for IPv4).
^ permalink raw reply
* Re: [PATCH] rfs: Receive Flow Steering
From: Eric Dumazet @ 2010-04-02 19:43 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <1270225735.11099.20.camel@edumazet-laptop>
Le vendredi 02 avril 2010 à 18:28 +0200, Eric Dumazet a écrit :
> Some more thoughts ...
>
> Do we really want to call inet_rps_record_flow(sk) from inet_sendmsg() &
> inet_sendpage() ?
>
> This seems not necessary to me...
>
I think I get it, you want to catch unidirectional flows (apps that only
send data), and let ACK packets be processed by the sender cpu :=)
I did following patch to remove one conditional branch :
net/core/dev.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 0a9ced8..cfe46d8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2225,8 +2225,6 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
u16 tcpu;
u32 addr1, addr2, ports, ihl;
- *rflowp = NULL;
-
if (skb_rx_queue_recorded(skb)) {
u16 index = skb_get_rx_queue(skb);
if (unlikely(index >= dev->num_rx_queues)) {
@@ -2443,7 +2441,7 @@ int netif_rx(struct sk_buff *skb)
{
unsigned int qtail;
#ifdef CONFIG_RPS
- struct rps_dev_flow *rflow;
+ struct rps_dev_flow voidflow, *rflow = &voidflow;
int cpu, err;
#endif
@@ -2461,10 +2459,7 @@ int netif_rx(struct sk_buff *skb)
if (cpu < 0)
cpu = smp_processor_id();
- err = enqueue_to_backlog(skb, cpu, &qtail);
-
- if (rflow)
- rflow->last_qtail = qtail;
+ err = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
rcu_read_unlock();
@@ -2839,7 +2834,7 @@ out:
int netif_receive_skb(struct sk_buff *skb)
{
#ifdef CONFIG_RPS
- struct rps_dev_flow *rflow;
+ struct rps_dev_flow voidflow, *rflow = &voidflow;
int cpu, err;
rcu_read_lock();
@@ -2848,13 +2843,8 @@ int netif_receive_skb(struct sk_buff *skb)
if (cpu < 0)
err = __netif_receive_skb(skb);
- else {
- unsigned int qtail;
-
- err = enqueue_to_backlog(skb, cpu, &qtail);
- if (rflow)
- rflow->last_qtail = qtail;
- }
+ else
+ err = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
rcu_read_unlock();
^ permalink raw reply related
* Re: Receive steering and hash and cache misses
From: Eric Dumazet @ 2010-04-02 19:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Tom Herbert, netdev
In-Reply-To: <20100402115439.348575c9@nehalam>
Le vendredi 02 avril 2010 à 11:54 -0700, Stephen Hemminger a écrit :
> On Fri, 2 Apr 2010 10:59:43 -0700
> Tom Herbert <therbert@google.com> wrote:
>
> > On Fri, Apr 2, 2010 at 10:26 AM, Stephen Hemminger
> > <shemminger@vyatta.com> wrote:
> > >
> > > Although Receive Packet Steering can use a hardware generated receive hash
> > > the device driver still causes an unnecessary cache miss on the interrupt
> > > processing CPU. The current Ethernet network device driver receive processing
> > > has the device driver calling eth_type_trans() which causes a the
> > > interrupt CPU to read the received frame header.
> > >
> >
> > It should be possible to deduce the values set by eth_type_trans from
> > the RX descriptor along with the RX hash. I'll post the patch getting
> > rxhash from bnx2x which does this.
> >
>
> On sky2, I get only RSS, Checksum, and length from descriptor info.
Doesnt sky2 also provide vlan id (OP_RXVLAN/OP_RXCHKSVLAN) ?
A future version of hardware could provide more info perhaps...
Must eth_type_trans() be done *before* netif_receive_skb() ?
If a device provides a rxhash, maybe we can delay eth_type_trans() too.
If not, we need to access IP header anyway in the first cpu.
^ permalink raw reply
* Re: Receive steering and hash and cache misses
From: Stephen Hemminger @ 2010-04-02 18:54 UTC (permalink / raw)
To: Tom Herbert; +Cc: Eric Dumazet, netdev
In-Reply-To: <g2l65634d661004021059z94214a43v82409d15a0fb09b6@mail.gmail.com>
On Fri, 2 Apr 2010 10:59:43 -0700
Tom Herbert <therbert@google.com> wrote:
> On Fri, Apr 2, 2010 at 10:26 AM, Stephen Hemminger
> <shemminger@vyatta.com> wrote:
> >
> > Although Receive Packet Steering can use a hardware generated receive hash
> > the device driver still causes an unnecessary cache miss on the interrupt
> > processing CPU. The current Ethernet network device driver receive processing
> > has the device driver calling eth_type_trans() which causes a the
> > interrupt CPU to read the received frame header.
> >
>
> It should be possible to deduce the values set by eth_type_trans from
> the RX descriptor along with the RX hash. I'll post the patch getting
> rxhash from bnx2x which does this.
>
On sky2, I get only RSS, Checksum, and length from descriptor info.
^ permalink raw reply
* Re: [PATCHv3] drivers/net/usb: Add new driver ipheth
From: Ben Hutchings @ 2010-04-02 18:35 UTC (permalink / raw)
To: "L. Alberto Giménez"
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
oliver-GvhC2dPhHPQdnm+yROfE0A, linville-2XuSBdqkA4R54TAoqtyWWQ,
j.dumon-x9gZzRpC1QbQT0dZR+AlfA, steve.glendinning-sdUf+H5yV5I,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, gregkh-l3A5Bk7waGM,
dgiagio-Re5JQEeQqe8AvxtiuMwx3w, Daniel Borca
In-Reply-To: <4BB62F33.1020507-lqZFv/KUvpAxAGwisGp4zA@public.gmane.org>
On Fri, 2010-04-02 at 19:53 +0200, "L. Alberto Giménez" wrote:
> On 04/02/2010 07:21 PM, Ben Hutchings wrote:
> > On Fri, 2010-04-02 at 19:15 +0200, "L. Alberto Giménez" wrote:
> >> On 04/01/2010 01:18 AM, Ben Hutchings wrote:
> >>> On Wed, 2010-03-31 at 21:42 +0200, L. Alberto Giménez wrote:
> >>> [...]
> >>>> --- /dev/null
> >>>> +++ b/drivers/net/usb/ipheth.c
> >>> [...]
> >>>> + usb_fill_bulk_urb(dev->tx_urb, udev,
> >>>> + usb_sndbulkpipe(udev, dev->bulk_out),
> >>>> + dev->tx_buf, IPHETH_BUF_SIZE,
> >>>> + ipheth_sndbulk_callback,
> >>>> + dev);
> >>>> + dev->tx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >>>> +
> >>>> + retval = usb_submit_urb(dev->tx_urb, GFP_ATOMIC);
> >>>> + if (retval) {
> >>>> + err("%s: usb_submit_urb: %d", __func__, retval);
> >>>> + dev->stats.tx_errors++;
> >>>> + dev_kfree_skb_irq(skb);
> >>>> + } else {
> >>>> + net->trans_start = jiffies;
> >>> No longer needed.
> >> What is not longer needed? The assignment, the whole "else" branch? If
> >> the assignment is what is not needed, can I just remove that line, right?
> >
> > The assignment is not needed.
>
> Hi,
>
> I've been looking into this and it seems that the net_device.trans_start
> field is now deprecated in favor of net_device.rx_queue.trans_start
[...]
The networking core updates netdev_queue::trans_start by calling
txq_trans_update() where necessary. Drivers should not touch either
net_device::trans_start or netdev_queue::trans_start any more.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] rfs: Receive Flow Steering
From: Rick Jones @ 2010-04-02 18:25 UTC (permalink / raw)
To: Tom Herbert; +Cc: Eric Dumazet, Changli Gao, davem, netdev
In-Reply-To: <g2i65634d661004021045uff7c0e25ge7dfd17929bc9ee9@mail.gmail.com>
Tom Herbert wrote:
>
>
> On Fri, Apr 2, 2010 at 10:01 AM, Rick Jones <rick.jones2@hp.com
> <mailto:rick.jones2@hp.com>> wrote:
>
> Eric Dumazet wrote:
>
>
> Your claim of RPS being not good for applications is wrong, our test
> results show an improvement as is. Maybe your applications dont
> scale,
> because of bad habits, or collidings heuristics, I dont know.
>
>
> The progression in HP-UX was IPS (10.20) (aka RPS) then TOPS (11.0)
> (aka RFS). We found that IPS was great for
> single-flow-per-thread-of-execution stuff and that TOPS was better
> for multiple-flow-per-thread-of-execution stuff. It was long enough
> ago now that I can safely say for one system-level benchmark not
> known to be a "networking" benchmark, and without a massive kernel
> component, TOPS was a 10% win. Not too shabby.
>
> It wasn't that IPS wasn't good in its context - just that TOPS was
> even better.
>
> I would assume that with IPS threads would migrate to where packets were
> being delivered thus giving the same sort of locality TOPS was
> providing? That would work great without any other constraints
> (multiple flows per thread, thread CPU bindings, etc.).
Well... that depended - at the time, and still, we were and are also encouraging
users and app designers to make copious use of processor/locality affinity (SMP
and NUMA going back far longer in the RISC et al space than the x86 space). So,
it was and is entirely possible that the application thread of execution is
hard-bound to a specific core/locality. Also, I do not recall if HP-UX was as
aggressive about waking a process/thread on the processor from which the wake-up
came vs on the processor on which it last ran.
> We also preferred the concept of the scheduler giving networking
> clues as to where to process an application's packets rather than
> networking trying to tell the scheduler. There was some discussion
> of out of order worries, but we were willing to trust to the basic
> soundness of the scheduler - if it was moving threads around willy
> nilly at a rate able to cause big packet reordering it had
> fundamental problems that would have to be addressed anyway.
>
>
> I also think scheduler leading networking, like in RPS, is generally
> more scalable. As for OOO packets, I've spent way to much time trying
> to convince the bean-counters that a small number of them aren't
> problematic :-), in the end it's just easier to not introduce new
> mechanisms that will cause them!
So long as it doesn't drive you to produce new mechanisms heavier than they
would have otherwise been.
The irony in the case of HP-UX IPS was that it was put in place in response to
the severe out of order packet problems in HP-UX in 10.X before 10.20 - there
were multiple netisr processes and only one netisr queue. The other little
tweak that came along in 10.20 with IPS, was inaddition to having a per
processor (well, per core in today's parlance) netisr queue, the netisr would
grab the entire queue under the one spinlock and work off of that. That was
nice because the code path became more efficient under load - more packets
processed per spinlock/unlock pair.
happy benchmarking,
rick jones
^ permalink raw reply
* Re: [PATCHv3] drivers/net/usb: Add new driver ipheth
From: "L. Alberto Giménez" @ 2010-04-02 18:23 UTC (permalink / raw)
To: Oliver Neukum
Cc: linux-kernel, netdev, linux-usb, linville, j.dumon,
steve.glendinning, davem, gregkh, dgiagio, dborca
In-Reply-To: <201003312233.26130.oliver@neukum.org>
On 03/31/2010 10:33 PM, Oliver Neukum wrote:
> Am Mittwoch, 31. März 2010 21:42:07 schrieb L. Alberto Giménez:
Hi Oliver,
Just like with Ben's comments I still have a couple of doubts about your
comments.
>> +
>> +static int ipheth_open(struct net_device *net)
>> +{
>> + struct ipheth_device *dev = netdev_priv(net);
>> + struct usb_device *udev = dev->udev;
>> + int retval = 0;
>> +
>> + usb_set_interface(udev, IPHETH_INTFNUM, IPHETH_ALT_INTFNUM);
>> + usb_clear_halt(udev, usb_rcvbulkpipe(udev, dev->bulk_in));
>> + usb_clear_halt(udev, usb_sndbulkpipe(udev, dev->bulk_out));
>
> Is this really needed? If so, please add a comment.
I understand that usb_clear_halt is only needed when the device has
transmitted data, and as it is "open" time, we can assume that no
transmissions ere made, so we don't need to clear anything (aka: remove
both lines), am I right?
>> +
>> + retval = ipheth_carrier_set(dev);
>> + if (retval)
>> + goto error;
>> +
>> + retval = ipheth_rx_submit(dev, GFP_KERNEL);
>> + if (retval)
>> + goto error;
>> +
>> + schedule_delayed_work(&dev->carrier_work, IPHETH_CARRIER_CHECK_TIMEOUT);
>
> Does it make sense to start rx while you have no carrier?
Well, I have no clue about this one. I think that upstream developers
should take a look into this (Dario, Daniel, could you?) since I don't
have the knowledge to decide what to do about it.
But I assume that as with the previous one, we have just opened the
device and we aren't (yet) doing anything with it, so we shouldn't start rx?
>> +static void ipheth_disconnect(struct usb_interface *intf)
>> +{
>> + struct ipheth_device *dev;
>> +
>> + dev = usb_get_intfdata(intf);
>> + if (dev != NULL) {
>
> is this check needed?
Does usb_get_infdata always return not NULL? I haven't found anything
about it (just manual pages for the function, but can't spot if it
cannot return NULL). We disconnected the device, but I understand that
the kernel still has the information and the allocated memory, so the
cleanup code is still needed, isn't it?
>
>> +static struct usb_driver ipheth_driver = {
>> + .name = "ipheth",
>> + .probe = ipheth_probe,
>> + .disconnect = ipheth_disconnect,
>> + .id_table = ipheth_table,
>> + .supports_autosuspend = 0,
>
> redundant
Why?
Thanks a lot in advance.
Best regards,
--
L. Alberto Giménez
JabberID agimenez@jabber.sysvalve.es
GnuPG key ID 0x3BAABDE1
^ permalink raw reply
* Re: [PATCH 5/5] netfilter: xt_TEE: have cloned packet travel through Xtables too
From: Jan Engelhardt @ 2010-04-02 18:15 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, netdev
In-Reply-To: <4BB4A796.8010708@trash.net>
On Thursday 2010-04-01 16:03, Patrick McHardy wrote:
>>>>[detecting teed packets getting teed again by means of
>>>> iptables -A OUTPUT -j TEE]
>>>
>>> What's wrong with adding a reentrancy counter?
>>
>> Sounds like a plan.
Should we be using a percpu variable, or is a simplistic
array ok too?
static bool tee_active;
target(...)
{
if (tee_active[smp_processor_id()])
return XT_CONTINUE;
...
if (tee_tg4_route(...)) {
tee_active[cpu] = true;
ip_local_out(skb);
tee_active[cpu] = false;
}
}
^ permalink raw reply
* Re: Any ideas about a crash on reboot with igb and intel_iommu?
From: Roland Dreier @ 2010-04-02 18:13 UTC (permalink / raw)
To: netdev; +Cc: iommu, David Woodhouse
In-Reply-To: <adabpe3f3c4.fsf@roland-alpha.cisco.com>
So actually I found the following change that went into 2.6.31:
commit 91615f765a2935b6cbae424b9eee1585ed681ae6
Author: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date: Tue Jun 30 12:45:15 2009 +0000
igb: fix unmap length bug
driver was mixing NET_IP_ALIGN count bytes in map/unmap calls
unevenly. Only map the bytes that the hardware might dma into
igb in 2.6.30.y is doing pci_map_single(<some length>) and doing
pci_unmap_single(<some other length>).
However I haven't been able to provoke the crash yet, even by bouncing
an igb interface (with VT-d turned on and with CONFIG_DMAR_DEFAULT_ON enabled).
Does this make sense as the sort of thing that might corrupt the iova
rbtree and lead to a crash in the __free_iova / rb_erase code?
Thanks,
Roland
--
Roland Dreier <rolandd@cisco.com> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
^ permalink raw reply
* Re: Receive steering and hash and cache misses
From: Tom Herbert @ 2010-04-02 17:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Eric Dumazet, netdev
In-Reply-To: <20100402102650.5bdb5b52@nehalam>
On Fri, Apr 2, 2010 at 10:26 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
>
> Although Receive Packet Steering can use a hardware generated receive hash
> the device driver still causes an unnecessary cache miss on the interrupt
> processing CPU. The current Ethernet network device driver receive processing
> has the device driver calling eth_type_trans() which causes a the
> interrupt CPU to read the received frame header.
>
It should be possible to deduce the values set by eth_type_trans from
the RX descriptor along with the RX hash. I'll post the patch getting
rxhash from bnx2x which does this.
> Is there some way the hardware receive hash value could be used to
> steer to the receive CPU, then have the receive CPU find the Ethernet
> type field (eth_type_trans)?
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv3] drivers/net/usb: Add new driver ipheth
From: "L. Alberto Giménez" @ 2010-04-02 17:53 UTC (permalink / raw)
To: Ben Hutchings
Cc: "L. Alberto Giménez", linux-kernel, netdev,
linux-usb, oliver, linville, j.dumon, steve.glendinning, davem,
gregkh, dgiagio, Daniel Borca
In-Reply-To: <1270228883.12516.140.camel@localhost>
On 04/02/2010 07:21 PM, Ben Hutchings wrote:
> On Fri, 2010-04-02 at 19:15 +0200, "L. Alberto Giménez" wrote:
>> On 04/01/2010 01:18 AM, Ben Hutchings wrote:
>>> On Wed, 2010-03-31 at 21:42 +0200, L. Alberto Giménez wrote:
>>> [...]
>>>> --- /dev/null
>>>> +++ b/drivers/net/usb/ipheth.c
>>> [...]
>>>> + usb_fill_bulk_urb(dev->tx_urb, udev,
>>>> + usb_sndbulkpipe(udev, dev->bulk_out),
>>>> + dev->tx_buf, IPHETH_BUF_SIZE,
>>>> + ipheth_sndbulk_callback,
>>>> + dev);
>>>> + dev->tx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>>>> +
>>>> + retval = usb_submit_urb(dev->tx_urb, GFP_ATOMIC);
>>>> + if (retval) {
>>>> + err("%s: usb_submit_urb: %d", __func__, retval);
>>>> + dev->stats.tx_errors++;
>>>> + dev_kfree_skb_irq(skb);
>>>> + } else {
>>>> + net->trans_start = jiffies;
>>> No longer needed.
>> What is not longer needed? The assignment, the whole "else" branch? If
>> the assignment is what is not needed, can I just remove that line, right?
>
> The assignment is not needed.
Hi,
I've been looking into this and it seems that the net_device.trans_start
field is now deprecated in favor of net_device.rx_queue.trans_start
-rx_queue is a struct netdev_queue- (file include/linux/netdevice.h), as
states the comment:
512 /*
513 * please use this field instead of dev->trans_start
514 */
515 unsigned long trans_start;
Reading LDD3 book, it says that the driver is reponsible for updating
trans_start (as well as trans_rx, but we're not talking about that one).
So, I guess that the LDD book is outdated (again ;) ), but what I don't
understand at all is wether the driver should keep updating the right
field (dev->rx_queue.trans_start) or if the fact that it's inside a
"queue" implies that the code that is responsible for that queue would
update the trans_start field by itself?
Thanks in advance.
Best regards,
--
L. Alberto Giménez
JabberID agimenez@jabber.sysvalve.es
GnuPG key ID 0x3BAABDE1
^ permalink raw reply
* Re: [PATCH 1/2] phylib: Support phy module autoloading
From: Andy Fleming @ 2010-04-02 17:51 UTC (permalink / raw)
To: David Woodhouse; +Cc: davem, netdev, ben
In-Reply-To: <1270206327.3101.2436.camel@macbook.infradead.org>
On Fri, Apr 2, 2010 at 6:05 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> We don't use the normal hotplug mechanism because it doesn't work. It will
> load the module some time after the device appears, but that's not good
> enough for us -- we need the driver loaded _immediately_ because otherwise
> the NIC driver may just abort and then the phy 'device' goes away.
>
> [bwh: s/phy/mdio/ in module alias, kerneldoc for struct mdio_device_id]
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Neat!
Acked-by: Andy Fleming <afleming@freescale.com>
^ permalink raw reply
* [PATCH] vhost: Make it more scalable by creating a vhost thread per device.
From: Sridhar Samudrala @ 2010-04-02 17:31 UTC (permalink / raw)
To: Michael S. Tsirkin, Tom Lendacky; +Cc: netdev, kvm@vger.kernel.org
Make vhost scalable by creating a separate vhost thread per vhost
device. This provides better scaling across multiple guests and with
multiple interfaces in a guest.
I am seeing better aggregated througput/latency when running netperf
across multiple guests or multiple interfaces in a guest in parallel
with this patch.
Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index a6a88df..29aa80f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -339,8 +339,10 @@ static int vhost_net_open(struct inode *inode, struct file *f)
return r;
}
- vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT);
- vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN);
+ vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT,
+ &n->dev);
+ vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN,
+ &n->dev);
n->tx_poll_state = VHOST_NET_POLL_DISABLED;
f->private_data = n;
@@ -643,25 +645,14 @@ static struct miscdevice vhost_net_misc = {
int vhost_net_init(void)
{
- int r = vhost_init();
- if (r)
- goto err_init;
- r = misc_register(&vhost_net_misc);
- if (r)
- goto err_reg;
- return 0;
-err_reg:
- vhost_cleanup();
-err_init:
- return r;
-
+ return misc_register(&vhost_net_misc);
}
+
module_init(vhost_net_init);
void vhost_net_exit(void)
{
misc_deregister(&vhost_net_misc);
- vhost_cleanup();
}
module_exit(vhost_net_exit);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 7bd7a1e..243f4d3 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -36,8 +36,6 @@ enum {
VHOST_MEMORY_F_LOG = 0x1,
};
-static struct workqueue_struct *vhost_workqueue;
-
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
{
@@ -56,18 +54,19 @@ static int vhost_poll_wakeup(wait_queue_t *wait, unsigned mode, int sync,
if (!((unsigned long)key & poll->mask))
return 0;
- queue_work(vhost_workqueue, &poll->work);
+ queue_work(poll->dev->wq, &poll->work);
return 0;
}
/* Init poll structure */
void vhost_poll_init(struct vhost_poll *poll, work_func_t func,
- unsigned long mask)
+ unsigned long mask, struct vhost_dev *dev)
{
INIT_WORK(&poll->work, func);
init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
init_poll_funcptr(&poll->table, vhost_poll_func);
poll->mask = mask;
+ poll->dev = dev;
}
/* Start polling a file. We add ourselves to file's wait queue. The caller must
@@ -96,7 +95,7 @@ void vhost_poll_flush(struct vhost_poll *poll)
void vhost_poll_queue(struct vhost_poll *poll)
{
- queue_work(vhost_workqueue, &poll->work);
+ queue_work(poll->dev->wq, &poll->work);
}
static void vhost_vq_reset(struct vhost_dev *dev,
@@ -128,6 +127,11 @@ long vhost_dev_init(struct vhost_dev *dev,
struct vhost_virtqueue *vqs, int nvqs)
{
int i;
+
+ dev->wq = create_singlethread_workqueue("vhost");
+ if (!dev->wq)
+ return -ENOMEM;
+
dev->vqs = vqs;
dev->nvqs = nvqs;
mutex_init(&dev->mutex);
@@ -143,7 +147,7 @@ long vhost_dev_init(struct vhost_dev *dev,
if (dev->vqs[i].handle_kick)
vhost_poll_init(&dev->vqs[i].poll,
dev->vqs[i].handle_kick,
- POLLIN);
+ POLLIN, dev);
}
return 0;
}
@@ -216,6 +220,8 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
if (dev->mm)
mmput(dev->mm);
dev->mm = NULL;
+
+ destroy_workqueue(dev->wq);
}
static int log_access_ok(void __user *log_base, u64 addr, unsigned long sz)
@@ -1095,16 +1101,3 @@ void vhost_disable_notify(struct vhost_virtqueue *vq)
vq_err(vq, "Failed to enable notification at %p: %d\n",
&vq->used->flags, r);
}
-
-int vhost_init(void)
-{
- vhost_workqueue = create_singlethread_workqueue("vhost");
- if (!vhost_workqueue)
- return -ENOMEM;
- return 0;
-}
-
-void vhost_cleanup(void)
-{
- destroy_workqueue(vhost_workqueue);
-}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 44591ba..60fefd0 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -29,10 +29,11 @@ struct vhost_poll {
/* struct which will handle all actual work. */
struct work_struct work;
unsigned long mask;
+ struct vhost_dev *dev;
};
void vhost_poll_init(struct vhost_poll *poll, work_func_t func,
- unsigned long mask);
+ unsigned long mask, struct vhost_dev *dev);
void vhost_poll_start(struct vhost_poll *poll, struct file *file);
void vhost_poll_stop(struct vhost_poll *poll);
void vhost_poll_flush(struct vhost_poll *poll);
@@ -110,6 +111,7 @@ struct vhost_dev {
int nvqs;
struct file *log_file;
struct eventfd_ctx *log_ctx;
+ struct workqueue_struct *wq;
};
long vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue *vqs, int nvqs);
@@ -136,9 +138,6 @@ bool vhost_enable_notify(struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
-int vhost_init(void);
-void vhost_cleanup(void);
-
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
if ((vq)->error_ctx) \
^ permalink raw reply related
* Receive steering and hash and cache misses
From: Stephen Hemminger @ 2010-04-02 17:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
Although Receive Packet Steering can use a hardware generated receive hash
the device driver still causes an unnecessary cache miss on the interrupt
processing CPU. The current Ethernet network device driver receive processing
has the device driver calling eth_type_trans() which causes a the
interrupt CPU to read the received frame header.
Is there some way the hardware receive hash value could be used to
steer to the receive CPU, then have the receive CPU find the Ethernet
type field (eth_type_trans)?
^ permalink raw reply
* Re: [PATCH] ethtool: RXHASH flag support (v2)
From: Jeff Garzik @ 2010-04-02 17:26 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20100402081515.1b963562@nehalam>
Both patches look great, thanks. Will apply after Easter holiday...
Jeff
^ permalink raw reply
* Re: [PATCHv3] drivers/net/usb: Add new driver ipheth
From: Ben Hutchings @ 2010-04-02 17:21 UTC (permalink / raw)
To: "L. Alberto Giménez"
Cc: linux-kernel, netdev, linux-usb, oliver, linville, j.dumon,
steve.glendinning, davem, gregkh, dgiagio, dborca
In-Reply-To: <4BB62620.3070807@sysvalve.es>
On Fri, 2010-04-02 at 19:15 +0200, "L. Alberto Giménez" wrote:
> On 04/01/2010 01:18 AM, Ben Hutchings wrote:
> > On Wed, 2010-03-31 at 21:42 +0200, L. Alberto Giménez wrote:
> > [...]
> >> --- /dev/null
> >> +++ b/drivers/net/usb/ipheth.c
> > [...]
>
> Hi Ben,
>
> Upstream has fixed several errors pointed out by you and Oliver (thanks
> for that), but some of them are still pending.
>
> I will send patches on top of my last driver submission (if the proper
> way would be resubmit the whole code, please tell me. Anyway I need to
> clarify some doubts...
Since David Miller has not merged your original patch, you should send a
single new patch with the changes incorporated.
> >> + usb_fill_bulk_urb(dev->tx_urb, udev,
> >> + usb_sndbulkpipe(udev, dev->bulk_out),
> >> + dev->tx_buf, IPHETH_BUF_SIZE,
> >> + ipheth_sndbulk_callback,
> >> + dev);
> >> + dev->tx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >> +
> >> + retval = usb_submit_urb(dev->tx_urb, GFP_ATOMIC);
> >> + if (retval) {
> >> + err("%s: usb_submit_urb: %d", __func__, retval);
> >> + dev->stats.tx_errors++;
> >> + dev_kfree_skb_irq(skb);
> >> + } else {
> >> + net->trans_start = jiffies;
> >
> > No longer needed.
>
> What is not longer needed? The assignment, the whole "else" branch? If
> the assignment is what is not needed, can I just remove that line, right?
The assignment is not needed.
> > [...]
> >> +#ifdef HAVE_NET_DEVICE_OPS
> >> +static const struct net_device_ops ipheth_netdev_ops = {
> >> + .ndo_open = &ipheth_open,
> >> + .ndo_stop = &ipheth_close,
> >> + .ndo_start_xmit = &ipheth_tx,
> >> + .ndo_tx_timeout = &ipheth_tx_timeout,
> >> + .ndo_get_stats = &ipheth_stats,
> >> +};
> >> +#endif
> >
> > Remove the #ifdef, there is no question whether we have net_device_ops.
>
> Ok, I will just remove both #ifdefs, but why is that? Maybe in previous
> versions of the kernel the net_device_ops struct was introduced and now
> it's present no matter how you configure your kernel?
[...]
Correct, it is now mandatory.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCHv3] drivers/net/usb: Add new driver ipheth
From: "L. Alberto Giménez" @ 2010-04-02 17:15 UTC (permalink / raw)
To: Ben Hutchings
Cc: linux-kernel, netdev, linux-usb, oliver, linville, j.dumon,
steve.glendinning, davem, gregkh, dgiagio, dborca
In-Reply-To: <1270077538.8653.484.camel@localhost>
On 04/01/2010 01:18 AM, Ben Hutchings wrote:
> On Wed, 2010-03-31 at 21:42 +0200, L. Alberto Giménez wrote:
> [...]
>> --- /dev/null
>> +++ b/drivers/net/usb/ipheth.c
> [...]
Hi Ben,
Upstream has fixed several errors pointed out by you and Oliver (thanks
for that), but some of them are still pending.
I will send patches on top of my last driver submission (if the proper
way would be resubmit the whole code, please tell me. Anyway I need to
clarify some doubts...
>> + usb_fill_bulk_urb(dev->tx_urb, udev,
>> + usb_sndbulkpipe(udev, dev->bulk_out),
>> + dev->tx_buf, IPHETH_BUF_SIZE,
>> + ipheth_sndbulk_callback,
>> + dev);
>> + dev->tx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>> +
>> + retval = usb_submit_urb(dev->tx_urb, GFP_ATOMIC);
>> + if (retval) {
>> + err("%s: usb_submit_urb: %d", __func__, retval);
>> + dev->stats.tx_errors++;
>> + dev_kfree_skb_irq(skb);
>> + } else {
>> + net->trans_start = jiffies;
>
> No longer needed.
What is not longer needed? The assignment, the whole "else" branch? If
the assignment is what is not needed, can I just remove that line, right?
> [...]
>> +#ifdef HAVE_NET_DEVICE_OPS
>> +static const struct net_device_ops ipheth_netdev_ops = {
>> + .ndo_open = &ipheth_open,
>> + .ndo_stop = &ipheth_close,
>> + .ndo_start_xmit = &ipheth_tx,
>> + .ndo_tx_timeout = &ipheth_tx_timeout,
>> + .ndo_get_stats = &ipheth_stats,
>> +};
>> +#endif
>
> Remove the #ifdef, there is no question whether we have net_device_ops.
Ok, I will just remove both #ifdefs, but why is that? Maybe in previous
versions of the kernel the net_device_ops struct was introduced and now
it's present no matter how you configure your kernel?
> I have no idea about USB so I haven't checked the USB API usage at all.
I think that Greg is the maintainer for the USB subsystem, so if he has
no further commets, I will try to submit fixes for both your and
Oliver's comments along with the upstream developers.
Thanks for your comments.
Best regards,
--
L. Alberto Giménez
JabberID agimenez@jabber.sysvalve.es
GnuPG key ID 0x3BAABDE1
^ permalink raw reply
* Re: [PATCH 0/2] Fix ethernet driver for Octeon based Movidis hardware
From: Greg KH @ 2010-04-02 17:04 UTC (permalink / raw)
To: David Miller; +Cc: ddaney, ralf, linux-mips, netdev
In-Reply-To: <20100401.182045.106908204.davem@davemloft.net>
On Thu, Apr 01, 2010 at 06:20:45PM -0700, David Miller wrote:
> From: David Daney <ddaney@caviumnetworks.com>
> Date: Thu, 1 Apr 2010 18:17:53 -0700
>
> > The Movidis X16 bootloader doesn't enable the mdio bus. The first
> > patch fixes this by enabling the mdio bus when the driver is
> > initialized.
> >
> > Also the addresses of the PHYs was unknown for this device. The
> > second patch adds the corresponding PHY addresses.
> >
> > With both patches applied I can successfully use all eight Ethernet
> > ports.
> >
> > Please consider for 2.6.34. Since Octeon is an embedded MIPS SOC it
> > is unlikely to break the kernel for any workstations. Any or all of
> > these could be considered for merging via Ralf's linux-mips.org tree.
>
> Ralf please merge this via your MIPS tree, thanks:
>
> Acked-by: David S. Miller <davem@davemloft.net>
I agree, Ralf, please take these and you can add:
Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
to them.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] rfs: Receive Flow Steering
From: Rick Jones @ 2010-04-02 17:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Changli Gao, Tom Herbert, davem, netdev
In-Reply-To: <1270193393.1936.52.camel@edumazet-laptop>
Eric Dumazet wrote:
>
> Your claim of RPS being not good for applications is wrong, our test
> results show an improvement as is. Maybe your applications dont scale,
> because of bad habits, or collidings heuristics, I dont know.
The progression in HP-UX was IPS (10.20) (aka RPS) then TOPS (11.0) (aka RFS).
We found that IPS was great for single-flow-per-thread-of-execution stuff and
that TOPS was better for multiple-flow-per-thread-of-execution stuff. It was
long enough ago now that I can safely say for one system-level benchmark not
known to be a "networking" benchmark, and without a massive kernel component,
TOPS was a 10% win. Not too shabby.
It wasn't that IPS wasn't good in its context - just that TOPS was even better.
We also preferred the concept of the scheduler giving networking clues as to
where to process an application's packets rather than networking trying to tell
the scheduler. There was some discussion of out of order worries, but we were
willing to trust to the basic soundness of the scheduler - if it was moving
threads around willy nilly at a rate able to cause big packet reordering it had
fundamental problems that would have to be addressed anyway. And while it may
be incindiary to point this out :) I suspect (without concrete data :) that
bonding mode 0 is a much, Much, MUCH larger source of out-of-order traffic than
any plausible scheduler thrashing.
happy benchmarking,
rick jones
^ permalink raw reply
* Re: [PATCH] rfs: Receive Flow Steering
From: Eric Dumazet @ 2010-04-02 16:28 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <1270211871.11099.2.camel@edumazet-laptop>
Some more thoughts ...
Do we really want to call inet_rps_record_flow(sk) from inet_sendmsg() &
inet_sendpage() ?
This seems not necessary to me...
This bit :
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index f4df5f9..3060c17 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1674,6 +1674,10 @@ process:
skb->dev = NULL;
+#ifdef CONFIG_RPS
+ inet_sk(sk)->rxhash = skb->rxhash;
+#endif
+
bh_lock_sock_nested(sk);
ret = 0;
if (!sock_owned_by_user(sk)) {
1) It's not pretty, could you define a helper ?
2) Why should we set inet_sk(sk)->rxhash at each packet ?
rxhash is a constant given the (src,dst,sport,dport) tuple and could be
set at connection time. This way we could set inet_sk(sk)->rxhash both
for TCP and UDP.
(Currently, you dont set inet_sk(sk)->rxhash on UDP)
^ permalink raw reply related
* Re: [PATCH v3 10/12] l2tp: Add L2TP ethernet pseudowire support
From: Paul E. McKenney @ 2010-04-02 16:21 UTC (permalink / raw)
To: Eric Dumazet; +Cc: James Chapman, Stephen Hemminger, netdev
In-Reply-To: <1270223931.11099.7.camel@edumazet-laptop>
On Fri, Apr 02, 2010 at 05:58:51PM +0200, Eric Dumazet wrote:
> Le vendredi 02 avril 2010 à 08:24 -0700, Paul E. McKenney a écrit :
> > On Wed, Mar 31, 2010 at 10:38:36AM -0700, Paul E. McKenney wrote:
>
> > > The list_is_last() is RCU-safe already, since the ->next pointer is
> > > only compared, never dereferenced. I suggest adding a comment to its
> > > header stating that it is RCU-safe.
> > >
> > > Feel free to create a list_for_each_entry_safe_rcu(), and I will be
> > > happy to review it.
> >
> > Right. When using RCU, list_for_each_entry_safe_rcu() is not needed,
> > because destruction of the element you removed doesn't happen until the
> > end of a subsequent grace period. By which time you will be done with
> > your RCU read-side critical section.
> >
> > So you can just use list_for_each_entry_rcu(), you do not need a new
> > list_for_each_entry_safe_rcu().
> >
> > But you did have me going for a bit!!! ;-)
>
> Just wondering Paul if you need to rest :)
In this case, I most certainly needed to have engaged my brain more
fully before hitting "send". :-/
> list_for_each_entry_safe_rcu() is not needed not because of various (and
> very intersting) reasons you gave !!!
>
> But because list_for_each_entry_safe() is OK, since if we are in a
> destroy phase, we are a writer, and can use regular list primitive.
>
> Remember _rcu() versions of list iterators are for readers, and readers
> are NOT supposed to delete an item from the list !
Yes, RCU readers had better have acquired the update-side lock before
modifying the list. And then there would be some debate as to whether
they were still readers. For example, while reading one list under RCU
protection, you might be modifying a subordinate list while holding the
corresponding lock.
And in the networking code, you would also need to convince the usual
suspects, including Eric, that upgrading to writer in the middle of an
RCU read-side critical section was the right thing to be doing in the
first place!
Thanx, Paul
^ permalink raw reply
* [PATCH net-next-2.6 v4 14/14] l2tp: Update documentation
From: James Chapman @ 2010-04-02 16:19 UTC (permalink / raw)
To: netdev
In-Reply-To: <20100402161822.11367.70454.stgit@bert.katalix.com>
This patch adds documentation about the L2TPv3 functionality.
Signed-off-by: James Chapman <jchapman@katalix.com>
Reviewed-by: Randy Dunlap <randy.dunlap@oracle.com>
---
Documentation/networking/l2tp.txt | 247 ++++++++++++++++++++++++++++++++-----
1 files changed, 213 insertions(+), 34 deletions(-)
diff --git a/Documentation/networking/l2tp.txt b/Documentation/networking/l2tp.txt
index 63214b2..e7bf397 100644
--- a/Documentation/networking/l2tp.txt
+++ b/Documentation/networking/l2tp.txt
@@ -1,44 +1,95 @@
-This brief document describes how to use the kernel's PPPoL2TP driver
-to provide L2TP functionality. L2TP is a protocol that tunnels one or
-more PPP sessions over a UDP tunnel. It is commonly used for VPNs
+This document describes how to use the kernel's L2TP drivers to
+provide L2TP functionality. L2TP is a protocol that tunnels one or
+more sessions over an IP tunnel. It is commonly used for VPNs
(L2TP/IPSec) and by ISPs to tunnel subscriber PPP sessions over an IP
-network infrastructure.
+network infrastructure. With L2TPv3, it is also useful as a Layer-2
+tunneling infrastructure.
+
+Features
+========
+
+L2TPv2 (PPP over L2TP (UDP tunnels)).
+L2TPv3 ethernet pseudowires.
+L2TPv3 PPP pseudowires.
+L2TPv3 IP encapsulation.
+Netlink sockets for L2TPv3 configuration management.
+
+History
+=======
+
+The original pppol2tp driver was introduced in 2.6.23 and provided
+L2TPv2 functionality (rfc2661). L2TPv2 is used to tunnel one or more PPP
+sessions over a UDP tunnel.
+
+L2TPv3 (rfc3931) changes the protocol to allow different frame types
+to be passed over an L2TP tunnel by moving the PPP-specific parts of
+the protocol out of the core L2TP packet headers. Each frame type is
+known as a pseudowire type. Ethernet, PPP, HDLC, Frame Relay and ATM
+pseudowires for L2TP are defined in separate RFC standards. Another
+change for L2TPv3 is that it can be carried directly over IP with no
+UDP header (UDP is optional). It is also possible to create static
+unmanaged L2TPv3 tunnels manually without a control protocol
+(userspace daemon) to manage them.
+
+To support L2TPv3, the original pppol2tp driver was split up to
+separate the L2TP and PPP functionality. Existing L2TPv2 userspace
+apps should be unaffected as the original pppol2tp sockets API is
+retained. L2TPv3, however, uses netlink to manage L2TPv3 tunnels and
+sessions.
Design
======
-The PPPoL2TP driver, drivers/net/pppol2tp.c, provides a mechanism by
-which PPP frames carried through an L2TP session are passed through
-the kernel's PPP subsystem. The standard PPP daemon, pppd, handles all
-PPP interaction with the peer. PPP network interfaces are created for
-each local PPP endpoint.
-
-The L2TP protocol http://www.faqs.org/rfcs/rfc2661.html defines L2TP
-control and data frames. L2TP control frames carry messages between
-L2TP clients/servers and are used to setup / teardown tunnels and
-sessions. An L2TP client or server is implemented in userspace and
-will use a regular UDP socket per tunnel. L2TP data frames carry PPP
-frames, which may be PPP control or PPP data. The kernel's PPP
+The L2TP protocol separates control and data frames. The L2TP kernel
+drivers handle only L2TP data frames; control frames are always
+handled by userspace. L2TP control frames carry messages between L2TP
+clients/servers and are used to setup / teardown tunnels and
+sessions. An L2TP client or server is implemented in userspace.
+
+Each L2TP tunnel is implemented using a UDP or L2TPIP socket; L2TPIP
+provides L2TPv3 IP encapsulation (no UDP) and is implemented using a
+new l2tpip socket family. The tunnel socket is typically created by
+userspace, though for unmanaged L2TPv3 tunnels, the socket can also be
+created by the kernel. Each L2TP session (pseudowire) gets a network
+interface instance. In the case of PPP, these interfaces are created
+indirectly by pppd using a pppol2tp socket. In the case of ethernet,
+the netdevice is created upon a netlink request to create an L2TPv3
+ethernet pseudowire.
+
+For PPP, the PPPoL2TP driver, net/l2tp/l2tp_ppp.c, provides a
+mechanism by which PPP frames carried through an L2TP session are
+passed through the kernel's PPP subsystem. The standard PPP daemon,
+pppd, handles all PPP interaction with the peer. PPP network
+interfaces are created for each local PPP endpoint. The kernel's PPP
subsystem arranges for PPP control frames to be delivered to pppd,
while data frames are forwarded as usual.
+For ethernet, the L2TPETH driver, net/l2tp/l2tp_eth.c, implements a
+netdevice driver, managing virtual ethernet devices, one per
+pseudowire. These interfaces can be managed using standard Linux tools
+such as "ip" and "ifconfig". If only IP frames are passed over the
+tunnel, the interface can be given an IP addresses of itself and its
+peer. If non-IP frames are to be passed over the tunnel, the interface
+can be added to a bridge using brctl. All L2TP datapath protocol
+functions are handled by the L2TP core driver.
+
Each tunnel and session within a tunnel is assigned a unique tunnel_id
and session_id. These ids are carried in the L2TP header of every
-control and data packet. The pppol2tp driver uses them to lookup
-internal tunnel and/or session contexts. Zero tunnel / session ids are
-treated specially - zero ids are never assigned to tunnels or sessions
-in the network. In the driver, the tunnel context keeps a pointer to
-the tunnel UDP socket. The session context keeps a pointer to the
-PPPoL2TP socket, as well as other data that lets the driver interface
-to the kernel PPP subsystem.
-
-Note that the pppol2tp kernel driver handles only L2TP data frames;
-L2TP control frames are simply passed up to userspace in the UDP
-tunnel socket. The kernel handles all datapath aspects of the
-protocol, including data packet resequencing (if enabled).
-
-There are a number of requirements on the userspace L2TP daemon in
-order to use the pppol2tp driver.
+control and data packet. (Actually, in L2TPv3, the tunnel_id isn't
+present in data frames - it is inferred from the IP connection on
+which the packet was received.) The L2TP driver uses the ids to lookup
+internal tunnel and/or session contexts to determine how to handle the
+packet. Zero tunnel / session ids are treated specially - zero ids are
+never assigned to tunnels or sessions in the network. In the driver,
+the tunnel context keeps a reference to the tunnel UDP or L2TPIP
+socket. The session context holds data that lets the driver interface
+to the kernel's network frame type subsystems, i.e. PPP, ethernet.
+
+Userspace Programming
+=====================
+
+For L2TPv2, there are a number of requirements on the userspace L2TP
+daemon in order to use the pppol2tp driver.
1. Use a UDP socket per tunnel.
@@ -86,6 +137,35 @@ In addition to the standard PPP ioctls, a PPPIOCGL2TPSTATS is provided
to retrieve tunnel and session statistics from the kernel using the
PPPoX socket of the appropriate tunnel or session.
+For L2TPv3, userspace must use the netlink API defined in
+include/linux/l2tp.h to manage tunnel and session contexts. The
+general procedure to create a new L2TP tunnel with one session is:-
+
+1. Open a GENL socket using L2TP_GENL_NAME for configuring the kernel
+ using netlink.
+
+2. Create a UDP or L2TPIP socket for the tunnel.
+
+3. Create a new L2TP tunnel using a L2TP_CMD_TUNNEL_CREATE
+ request. Set attributes according to desired tunnel parameters,
+ referencing the UDP or L2TPIP socket created in the previous step.
+
+4. Create a new L2TP session in the tunnel using a
+ L2TP_CMD_SESSION_CREATE request.
+
+The tunnel and all of its sessions are closed when the tunnel socket
+is closed. The netlink API may also be used to delete sessions and
+tunnels. Configuration and status info may be set or read using netlink.
+
+The L2TP driver also supports static (unmanaged) L2TPv3 tunnels. These
+are where there is no L2TP control message exchange with the peer to
+setup the tunnel; the tunnel is configured manually at each end of the
+tunnel. There is no need for an L2TP userspace application in this
+case -- the tunnel socket is created by the kernel and configured
+using parameters sent in the L2TP_CMD_TUNNEL_CREATE netlink
+request. The "ip" utility of iproute2 has commands for managing static
+L2TPv3 tunnels; do "ip l2tp help" for more information.
+
Debugging
=========
@@ -102,6 +182,69 @@ PPPOL2TP_MSG_CONTROL userspace - kernel interface
PPPOL2TP_MSG_SEQ sequence numbers handling
PPPOL2TP_MSG_DATA data packets
+If enabled, files under a l2tp debugfs directory can be used to dump
+kernel state about L2TP tunnels and sessions. To access it, the
+debugfs filesystem must first be mounted.
+
+# mount -t debugfs debugfs /debug
+
+Files under the l2tp directory can then be accessed.
+
+# cat /debug/l2tp/tunnels
+
+The debugfs files should not be used by applications to obtain L2TP
+state information because the file format is subject to change. It is
+implemented to provide extra debug information to help diagnose
+problems.) Users should use the netlink API.
+
+/proc/net/pppol2tp is also provided for backwards compaibility with
+the original pppol2tp driver. It lists information about L2TPv2
+tunnels and sessions only. Its use is discouraged.
+
+Unmanaged L2TPv3 Tunnels
+========================
+
+Some commercial L2TP products support unmanaged L2TPv3 ethernet
+tunnels, where there is no L2TP control protocol; tunnels are
+configured at each side manually. New commands are available in
+iproute2's ip utility to support this.
+
+To create an L2TPv3 ethernet pseudowire between local host 192.168.1.1
+and peer 192.168.1.2, using IP addresses 10.5.1.1 and 10.5.1.2 for the
+tunnel endpoints:-
+
+# modprobe l2tp_eth
+# modprobe l2tp_netlink
+
+# ip l2tp add tunnel tunnel_id 1 peer_tunnel_id 1 udp_sport 5000 \
+ udp_dport 5000 encap udp local 192.168.1.1 remote 192.168.1.2
+# ip l2tp add session tunnel_id 1 session_id 1 peer_session_id 1
+# ifconfig -a
+# ip addr add 10.5.1.2/32 peer 10.5.1.1/32 dev l2tpeth0
+# ifconfig l2tpeth0 up
+
+Choose IP addresses to be the address of a local IP interface and that
+of the remote system. The IP addresses of the l2tpeth0 interface can be
+anything suitable.
+
+Repeat the above at the peer, with ports, tunnel/session ids and IP
+addresses reversed. The tunnel and session IDs can be any non-zero
+32-bit number, but the values must be reversed at the peer.
+
+Host 1 Host2
+udp_sport=5000 udp_sport=5001
+udp_dport=5001 udp_dport=5000
+tunnel_id=42 tunnel_id=45
+peer_tunnel_id=45 peer_tunnel_id=42
+session_id=128 session_id=5196755
+peer_session_id=5196755 peer_session_id=128
+
+When done at both ends of the tunnel, it should be possible to send
+data over the network. e.g.
+
+# ping 10.5.1.1
+
+
Sample Userspace Code
=====================
@@ -158,12 +301,48 @@ Sample Userspace Code
}
return 0;
+Internal Implementation
+=======================
+
+The driver keeps a struct l2tp_tunnel context per L2TP tunnel and a
+struct l2tp_session context for each session. The l2tp_tunnel is
+always associated with a UDP or L2TP/IP socket and keeps a list of
+sessions in the tunnel. The l2tp_session context keeps kernel state
+about the session. It has private data which is used for data specific
+to the session type. With L2TPv2, the session always carried PPP
+traffic. With L2TPv3, the session can also carry ethernet frames
+(ethernet pseudowire) or other data types such as ATM, HDLC or Frame
+Relay.
+
+When a tunnel is first opened, the reference count on the socket is
+increased using sock_hold(). This ensures that the kernel socket
+cannot be removed while L2TP's data structures reference it.
+
+Some L2TP sessions also have a socket (PPP pseudowires) while others
+do not (ethernet pseudowires). We can't use the socket reference count
+as the reference count for session contexts. The L2TP implementation
+therefore has its own internal reference counts on the session
+contexts.
+
+To Do
+=====
+
+Add L2TP tunnel switching support. This would route tunneled traffic
+from one L2TP tunnel into another. Specified in
+http://tools.ietf.org/html/draft-ietf-l2tpext-tunnel-switching-08
+
+Add L2TPv3 VLAN pseudowire support.
+
+Add L2TPv3 IP pseudowire support.
+
+Add L2TPv3 ATM pseudowire support.
+
Miscellaneous
-============
+=============
-The PPPoL2TP driver was developed as part of the OpenL2TP project by
+The L2TP drivers were developed as part of the OpenL2TP project by
Katalix Systems Ltd. OpenL2TP is a full-featured L2TP client / server,
designed from the ground up to have the L2TP datapath in the
kernel. The project also implemented the pppol2tp plugin for pppd
which allows pppd to use the kernel driver. Details can be found at
-http://openl2tp.sourceforge.net.
+http://www.openl2tp.org.
^ permalink raw reply related
* [PATCH net-next-2.6 v4 13/14] l2tp: Add support for static unmanaged L2TPv3 tunnels
From: James Chapman @ 2010-04-02 16:19 UTC (permalink / raw)
To: netdev
In-Reply-To: <20100402161822.11367.70454.stgit@bert.katalix.com>
This patch adds support for static (unmanaged) L2TPv3 tunnels, where
the tunnel socket is created by the kernel rather than being created
by userspace. This means L2TP tunnels and sessions can be created
manually, without needing an L2TP control protocol implemented in
userspace. This might be useful where the user wants a simple ethernet
over IP tunnel.
A patch to iproute2 adds a new command set under "ip l2tp" to make use
of this feature. This will be submitted separately.
Signed-off-by: James Chapman <jchapman@katalix.com>
Reviewed-by: Randy Dunlap <randy.dunlap@oracle.com>
---
net/l2tp/l2tp_core.c | 115 +++++++++++++++++++++++++++++++++++++++++++----
net/l2tp/l2tp_core.h | 7 +++
net/l2tp/l2tp_netlink.c | 18 ++++++-
3 files changed, 126 insertions(+), 14 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 473cf2d..13ed85b 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1218,6 +1218,82 @@ void l2tp_tunnel_free(struct l2tp_tunnel *tunnel)
}
EXPORT_SYMBOL_GPL(l2tp_tunnel_free);
+/* Create a socket for the tunnel, if one isn't set up by
+ * userspace. This is used for static tunnels where there is no
+ * managing L2TP daemon.
+ */
+static int l2tp_tunnel_sock_create(u32 tunnel_id, u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg, struct socket **sockp)
+{
+ int err = -EINVAL;
+ struct sockaddr_in udp_addr;
+ struct sockaddr_l2tpip ip_addr;
+ struct socket *sock;
+
+ switch (cfg->encap) {
+ case L2TP_ENCAPTYPE_UDP:
+ err = sock_create(AF_INET, SOCK_DGRAM, 0, sockp);
+ if (err < 0)
+ goto out;
+
+ sock = *sockp;
+
+ memset(&udp_addr, 0, sizeof(udp_addr));
+ udp_addr.sin_family = AF_INET;
+ udp_addr.sin_addr = cfg->local_ip;
+ udp_addr.sin_port = htons(cfg->local_udp_port);
+ err = kernel_bind(sock, (struct sockaddr *) &udp_addr, sizeof(udp_addr));
+ if (err < 0)
+ goto out;
+
+ udp_addr.sin_family = AF_INET;
+ udp_addr.sin_addr = cfg->peer_ip;
+ udp_addr.sin_port = htons(cfg->peer_udp_port);
+ err = kernel_connect(sock, (struct sockaddr *) &udp_addr, sizeof(udp_addr), 0);
+ if (err < 0)
+ goto out;
+
+ if (!cfg->use_udp_checksums)
+ sock->sk->sk_no_check = UDP_CSUM_NOXMIT;
+
+ break;
+
+ case L2TP_ENCAPTYPE_IP:
+ err = sock_create(AF_INET, SOCK_DGRAM, IPPROTO_L2TP, sockp);
+ if (err < 0)
+ goto out;
+
+ sock = *sockp;
+
+ memset(&ip_addr, 0, sizeof(ip_addr));
+ ip_addr.l2tp_family = AF_INET;
+ ip_addr.l2tp_addr = cfg->local_ip;
+ ip_addr.l2tp_conn_id = tunnel_id;
+ err = kernel_bind(sock, (struct sockaddr *) &ip_addr, sizeof(ip_addr));
+ if (err < 0)
+ goto out;
+
+ ip_addr.l2tp_family = AF_INET;
+ ip_addr.l2tp_addr = cfg->peer_ip;
+ ip_addr.l2tp_conn_id = peer_tunnel_id;
+ err = kernel_connect(sock, (struct sockaddr *) &ip_addr, sizeof(ip_addr), 0);
+ if (err < 0)
+ goto out;
+
+ break;
+
+ default:
+ goto out;
+ }
+
+out:
+ if ((err < 0) && sock) {
+ sock_release(sock);
+ *sockp = NULL;
+ }
+
+ return err;
+}
+
int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg, struct l2tp_tunnel **tunnelp)
{
struct l2tp_tunnel *tunnel = NULL;
@@ -1228,14 +1304,21 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
enum l2tp_encap_type encap = L2TP_ENCAPTYPE_UDP;
/* Get the tunnel socket from the fd, which was opened by
- * the userspace L2TP daemon.
+ * the userspace L2TP daemon. If not specified, create a
+ * kernel socket.
*/
- err = -EBADF;
- sock = sockfd_lookup(fd, &err);
- if (!sock) {
- printk(KERN_ERR "tunl %hu: sockfd_lookup(fd=%d) returned %d\n",
- tunnel_id, fd, err);
- goto err;
+ if (fd < 0) {
+ err = l2tp_tunnel_sock_create(tunnel_id, peer_tunnel_id, cfg, &sock);
+ if (err < 0)
+ goto err;
+ } else {
+ err = -EBADF;
+ sock = sockfd_lookup(fd, &err);
+ if (!sock) {
+ printk(KERN_ERR "tunl %hu: sockfd_lookup(fd=%d) returned %d\n",
+ tunnel_id, fd, err);
+ goto err;
+ }
}
sk = sock->sk;
@@ -1329,7 +1412,10 @@ err:
if (tunnelp)
*tunnelp = tunnel;
- if (sock)
+ /* If tunnel's socket was created by the kernel, it doesn't
+ * have a file.
+ */
+ if (sock && sock->file)
sockfd_put(sock);
return err;
@@ -1341,13 +1427,22 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_create);
int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
{
int err = 0;
+ struct socket *sock = tunnel->sock ? tunnel->sock->sk_socket : NULL;
/* Force the tunnel socket to close. This will eventually
* cause the tunnel to be deleted via the normal socket close
* mechanisms when userspace closes the tunnel socket.
*/
- if ((tunnel->sock != NULL) && (tunnel->sock->sk_socket != NULL))
- err = inet_shutdown(tunnel->sock->sk_socket, 2);
+ if (sock != NULL) {
+ err = inet_shutdown(sock, 2);
+
+ /* If the tunnel's socket was created by the kernel,
+ * close the socket here since the socket was not
+ * created by userspace.
+ */
+ if (sock->file == NULL)
+ err = inet_release(sock);
+ }
return err;
}
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 5713355..a961c77 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -146,6 +146,13 @@ struct l2tp_tunnel_cfg {
int debug; /* bitmask of debug message
* categories */
enum l2tp_encap_type encap;
+
+ /* Used only for kernel-created sockets */
+ struct in_addr local_ip;
+ struct in_addr peer_ip;
+ u16 local_udp_port;
+ u16 peer_udp_port;
+ int use_udp_checksums:1;
};
struct l2tp_tunnel {
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 3d0f7f6..12341a6 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -129,11 +129,21 @@ static int l2tp_nl_cmd_tunnel_create(struct sk_buff *skb, struct genl_info *info
}
cfg.encap = nla_get_u16(info->attrs[L2TP_ATTR_ENCAP_TYPE]);
- if (!info->attrs[L2TP_ATTR_FD]) {
- ret = -EINVAL;
- goto out;
+ fd = -1;
+ if (info->attrs[L2TP_ATTR_FD]) {
+ fd = nla_get_u32(info->attrs[L2TP_ATTR_FD]);
+ } else {
+ if (info->attrs[L2TP_ATTR_IP_SADDR])
+ cfg.local_ip.s_addr = nla_get_be32(info->attrs[L2TP_ATTR_IP_SADDR]);
+ if (info->attrs[L2TP_ATTR_IP_DADDR])
+ cfg.peer_ip.s_addr = nla_get_be32(info->attrs[L2TP_ATTR_IP_DADDR]);
+ if (info->attrs[L2TP_ATTR_UDP_SPORT])
+ cfg.local_udp_port = nla_get_u16(info->attrs[L2TP_ATTR_UDP_SPORT]);
+ if (info->attrs[L2TP_ATTR_UDP_DPORT])
+ cfg.peer_udp_port = nla_get_u16(info->attrs[L2TP_ATTR_UDP_DPORT]);
+ if (info->attrs[L2TP_ATTR_UDP_CSUM])
+ cfg.use_udp_checksums = nla_get_flag(info->attrs[L2TP_ATTR_UDP_CSUM]);
}
- fd = nla_get_u32(info->attrs[L2TP_ATTR_FD]);
if (info->attrs[L2TP_ATTR_DEBUG])
cfg.debug = nla_get_u32(info->attrs[L2TP_ATTR_DEBUG]);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox