S2io: two buffer mode

PATCH In review Ravinandan Arakali 7 participants 16 messages Oct 14, 2004 – Oct 18, 2004

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Ravinandan Arakali

author

2004-10-14 01:18 UTC #0 lore raw

PATCH 8/8 S2io: two buffer mode

Hi,
Attached is the patch for implementing 2-buffer mode on Rx path. More description of this mode of operation follows.

On certain systems when a DMA has to happen on an un-aligned memory location performance will take a significant hit. It's standard practice to offset the Rx buffer address by 2 (as Mac header is 14 bytes) so the IP header starts from an aligned location. Obviously using a single Rx buffer both cannot be achieved. Thus XFrame supports something called 2 buffer Rx mode, where in the Rx'ed frame is split into 2 parts, one is the Ethernet header and the other is the Ethernet payload. So now we can allocate proper aligned memory for both buffers, hence the DMA is not slowed down. Also, the Ethernet payload(starting from L3 header) is on an aligned location so OS need not have to do un-aligned accesses to process IP header. To achieve this, the kernel function eth_type_trans functionality has to be partially implemented in the driver itself.

Signed-off-by Raghavendra Koushik <[email protected]>

Jeff Garzik

2004-10-14 15:03 UTC #1 lore raw

no objections but two comments:

1) can this not be done with SKB fragments?

2) the following code needlessly uses GFP_ATOMIC:

Review

Quoted 4 lines

(quoted diff) +4 -0

+                               ba = &nic->ba[i][j][k];
+
+                               ba->ba_0_org = (void *) kmalloc
+                                   (BUF0_LEN + ALIGN_SIZE, GFP_ATOMIC);

and there is another example just a bit down from that one.

Raghavendra Koushik

2004-10-14 18:33 UTC #2 lore raw

Quoted 9 lines

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Thursday, October 14, 2004 8:04 AM
To: [email protected]
Cc: 'Francois Romieu'; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 2.6.9-rc2 8/8] S2io: two buffer mode

Quoted 4 lines

no objections but two comments:
1) can this not be done with SKB fragments?

To acheve what is required, the H/W still has to split the Rx'ed frame into 2 parts. By implementing this through the SKB fragments method, the only thing we avoid is usage of ba_0 and ba_1 fields, but note that none of these are allocated or freed in fast path. They are all pre-allocated buffers (along with the Rx descriptors) and hence won't hit the Rx side performance. Also by pulling down the eth_type_trans implementation partially into the driver any kind of copy during Rx is also avoided.

Quoted 8 lines

2) the following code needlessly uses GFP_ATOMIC:
Review
Quoted 4 lines
(quoted diff) +4 -0
+                               ba = &nic->ba[i][j][k];
+
+                               ba->ba_0_org = (void *) kmalloc
+                                   (BUF0_LEN + ALIGN_SIZE, GFP_ATOMIC);
and there is another example just a bit down from that one.

Chris Leech

2004-10-14 20:49 UTC #3 lore raw

Quoted 9 lines · Raghavendra Koushik

Quoted 2 lines
1) can this not be done with SKB fragments?
To acheve what is required, the H/W still has to split the Rx'ed frame into 2 parts. By implementing this through the SKB fragments method, the only thing we avoid is usage of ba_0 and ba_1 fields, but note that none of these are allocated or freed in fast path. They are all pre-allocated buffers (along with the Rx descriptors) and hence won't hit the Rx side performance. Also by pulling down the eth_type_trans implementation partially into the driver any kind of copy during Rx is also avoided.

This change would pass skbs from the driver without the MAC header? What happens in a bridging configuration? And dev->hard_header_parse is set to eth_header_parse, which expects skb->mac.ethernet to be valid, so I think any use of packet sockets will result in a NULL dereference.

- Chris

Raghavendra Koushik

2004-10-15 00:02 UTC #4 lore raw

Hi Chris,

I think you have a point. As an alternative I can allocate some
extra buffer space for buffer 2 (Ethernet payload) to begin with and in Rx path copy the buffer 1(Ethernet header) to the start of buffer 2(skb_push). This can then be passed to the upper layer (after eth_type_trans call). I did try fragmented SKB on Rx but it didn't seem to work. If any one can give me some pointers on how to get this working, I would implement 2Buff mode using this method and avoid a copy in fast path.

Koushik

Quoted 32 lines

-----Original Message-----
From: Chris Leech [mailto:[email protected]]
Sent: Thursday, October 14, 2004 1:49 PM
To: Raghavendra Koushik
Cc: Jeff Garzik; [email protected]; Francois Romieu; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 2.6.9-rc2 8/8] S2io: two buffer mode
Quoted 3 lines · Raghavendra Koushik
Quoted 2 lines
1) can this not be done with SKB fragments?
To acheve what is required, the H/W still has to split the Rx'ed frame
into
Quoted 2 lines
2 parts. By implementing this through the SKB fragments method, the only thing we avoid is usage of ba_0 and ba_1 fields, but note that none of
these
Quoted 2 lines
are allocated or freed in fast path. They are all pre-allocated buffers (along with the Rx descriptors) and hence won't hit the Rx side
performance.
Quoted 1 line
Also by pulling down the eth_type_trans implementation partially into
the
Quoted 1 line
driver any kind of copy during Rx is also avoided.
This change would pass skbs from the driver without the MAC header? What happens in a bridging configuration? And dev->hard_header_parse is set to eth_header_parse, which expects skb->mac.ethernet to be valid, so I think any use of packet sockets will result in a NULL dereference.
- Chris

Leonid Grossman

2004-10-15 00:21 UTC #5 lore raw

"fragmented SKB on Rx" is an old problem - we tried to deploy 2 and 5
buffer Xframe modes before and it did not work in Linux.
Is anyone aware of any network driver that successfully uses fragmented SKBs?

Thanks, Leonid

Quoted 66 lines

-----Original Message-----
From: Raghavendra Koushik [mailto:[email protected]]
Sent: Thursday, October 14, 2004 5:03 PM
To: 'Chris Leech'
Cc: 'Jeff Garzik'; [email protected]; 'Francois Romieu'; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH 2.6.9-rc2 8/8] S2io: two buffer mode
Hi Chris,
I think you have a point. As an alternative I can
allocate some extra buffer space for buffer 2 (Ethernet payload) to begin with and in Rx path copy the buffer 1(Ethernet header) to the start of buffer 2(skb_push). This can then be passed to the upper layer (after eth_type_trans call).
I did try fragmented SKB on Rx but it didn't seem to work. If any one can give me some pointers on how to get this working, I would implement 2Buff mode using this method and avoid a copy in fast path.
Koushik
Quoted 8 lines
-----Original Message-----
From: Chris Leech [mailto:[email protected]]
Sent: Thursday, October 14, 2004 1:49 PM
To: Raghavendra Koushik
Cc: Jeff Garzik; [email protected]; Francois Romieu; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 2.6.9-rc2 8/8] S2io: two buffer mode
Quoted 9 lines
Quoted 4 lines · Raghavendra Koushik
Quoted 2 lines
1) can this not be done with SKB fragments?
To acheve what is required, the H/W still has to split the Rx'ed frame
into
Quoted 1 line
2 parts. By implementing this through the SKB fragments
method, the
Quoted 1 line
Quoted 1 line
only thing we avoid is usage of ba_0 and ba_1 fields, but
note that
Quoted 4 lines
Quoted 1 line
none of
these
Quoted 2 lines
are allocated or freed in fast path. They are all pre-allocated buffers (along with the Rx descriptors) and hence won't
hit the Rx
Quoted 9 lines
Quoted 1 line
side
performance.
Quoted 2 lines
Also by pulling down the eth_type_trans implementation partially into
the
Quoted 1 line
driver any kind of copy during Rx is also avoided.
This change would pass skbs from the driver without the MAC header? What happens in a bridging configuration? And
dev->hard_header_parse
Quoted 5 lines
is set to eth_header_parse, which expects skb->mac.ethernet to be valid, so I think any use of packet sockets will result in a NULL dereference.
- Chris

Jeff Garzik

2004-10-15 18:47 UTC #6 lore raw

Quoted 3 lines · Leonid Grossman

"fragmented SKB on Rx" is an old problem - we tried to deploy 2 and 5
buffer Xframe modes before and it did not work in Linux. Is anyone aware of any network driver that successfully uses fragmented SKBs?

Quoted 3 lines · Raghavendra Koushik

I did try fragmented SKB on Rx but it didn't seem to work. If any one can give me some pointers on how to get this working, I would implement 2Buff mode using this method and avoid a copy in fast path.

My suggestion would be to investigate why fragmented SKBs don't work ;-)

The source code is all there, debug away...

If S2IO can fix this, a lot of people will benefit.

Jeff

Leonid Grossman

2004-10-15 19:48 UTC #7 lore raw

OK, we will debug and try to fix this.
Leonid

Quoted 35 lines

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Friday, October 15, 2004 11:47 AM
To: Leonid Grossman
Cc: 'Raghavendra Koushik'; 'Chris Leech'; [email protected]; 'Francois Romieu'; [email protected]; [email protected]
Subject: Re: [PATCH 2.6.9-rc2 8/8] S2io: two buffer mode
Quoted 1 line · Leonid Grossman
"fragmented SKB on Rx" is an old problem - we tried to
deploy 2 and 5
Quoted 3 lines
buffer Xframe modes before and it did not work in Linux. Is anyone aware of any network driver that successfully uses fragmented SKBs?
Quoted 1 line · Raghavendra Koushik
I did try fragmented SKB on Rx but it didn't seem to work.
If any one
Quoted 2 lines
can give me some pointers on how to get this working, I would implement 2Buff mode using this method and avoid a copy in
fast path.
My suggestion would be to investigate why fragmented SKBs don't work ;-)
The source code is all there, debug away...
If S2IO can fix this, a lot of people will benefit.
Jeff

Andi Kleen

2004-10-15 19:57 UTC #8 lore raw

Quoted 1 line · Leonid Grossman

OK, we will debug and try to fix this.

iirc it was originally implemented for HIPPI because the MM system had some problems with allocating 64K linear buffers reliably. But I'm not sure anybody still uses the HIPPI driver so it may have been a bit bitrotted.

-Andi

Raghavendra Koushik

2004-10-15 23:14 UTC #9 lore raw

In reply to Jeff Garzik

Jeff,
In this submission I'll use copy for 2Buff mode, but continue to debug the
fragmented skb Rx. If we do get it working sometime, I'll submit another patch to update the driver. Is this Ok?

- Koushik

Quoted 30 lines

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Friday, October 15, 2004 11:47 AM
To: Leonid Grossman
Cc: 'Raghavendra Koushik'; 'Chris Leech'; [email protected]; 'Francois Romieu'; [email protected]; [email protected]
Subject: Re: [PATCH 2.6.9-rc2 8/8] S2io: two buffer mode
Quoted 4 lines · Leonid Grossman
"fragmented SKB on Rx" is an old problem - we tried to deploy 2 and 5
buffer Xframe modes before and it did not work in Linux.
Is anyone aware of any network driver that successfully uses fragmented SKBs?
Quoted 1 line · Raghavendra Koushik
I did try fragmented SKB on Rx but it didn't seem to work. If any one
can
Quoted 1 line
give me some pointers on how to get this working, I would implement
2Buff
Quoted 1 line
mode using this method and avoid a copy in fast path.
My suggestion would be to investigate why fragmented SKBs don't work ;-)
The source code is all there, debug away...
If S2IO can fix this, a lot of people will benefit.
Jeff

Jeff Garzik

2004-10-15 23:15 UTC #10 lore raw

Quoted 4 lines · Raghavendra Koushik

Jeff,
In this submission I'll use copy for 2Buff mode, but continue to debug the
fragmented skb Rx. If we do get it working sometime, I'll submit another patch to update the driver. Is this Ok?

Sounds good to me.

Jeff

Jeff Garzik

2004-10-14 15:05 UTC #11 lore raw

s2io: patch disposition

In reply to Ravinandan Arakali

Ok, I have sent all my comments for your patches.

All patches must be resend, since your mailer mangled them.

It is up to you whether you wish to modify these patches for my comments, or create additional patches that address my comments. Your choice.

Jeff

Ravinandan Arakali

author

2004-10-18 22:06 UTC #12 lore raw

RE: s2io: patch disposition

Jeff,
We'll resend the original 8 patches, followed by patch to address your comments. Is it okay to send a single patch to address your comments or you want that to be split as well ?

Thanks,
Ravi

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Thursday, October 14, 2004 8:05 AM
To: [email protected]
Cc: 'Francois Romieu'; [email protected]; [email protected]; [email protected]; [email protected]
Subject: s2io: patch disposition

Ok, I have sent all my comments for your patches.

All patches must be resend, since your mailer mangled them.

It is up to you whether you wish to modify these patches for my comments, or create additional patches that address my comments. Your choice.

Jeff

Jeff Garzik

2004-10-18 22:04 UTC #13 lore raw

Re: s2io: patch disposition

Quoted 4 lines · Ravinandan Arakali

Jeff,
We'll resend the original 8 patches, followed by patch to address your comments. Is it okay to send a single patch to address your comments or you want that to be split as well ?

It depends on the changes. The rule is "separate logical changes"

Jeff

Francois Romieu

2004-10-18 22:21 UTC #14 lore raw

Re: s2io: patch disposition

In reply to Ravinandan Arakali

Ravinandan Arakali <[email protected]> :
[...]

Quoted 3 lines

We'll resend the original 8 patches, followed by patch to address your comments. Is it okay to send a single patch to address your comments or you want that to be split as well ?

Dumb question: is there something which prevents your patches to be sent with an MUA which inlines patches ?

Or is there any hope for you to run some Perl crap on your computer ? :o)

--
Ueimor

Ravinandan Arakali

author

2004-10-18 22:38 UTC #15 lore raw

RE: s2io: patch disposition

Hi Francois,
When we resend the patches, they will be inlined in the e-mail.

Ravi

-----Original Message-----
From: Francois Romieu [mailto:[email protected]]
Sent: Monday, October 18, 2004 3:21 PM
To: Ravinandan Arakali
Cc: Jeff Garzik; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: s2io: patch disposition

Ravinandan Arakali <[email protected]> :
[...]

Quoted 3 lines

We'll resend the original 8 patches, followed by patch to address your comments. Is it okay to send a single patch to address your comments or you want that to be split as well ?

Dumb question: is there something which prevents your patches to be sent with an MUA which inlines patches ?

Or is there any hope for you to run some Perl crap on your computer ? :o)

--
Ueimor