Skip to contentSkip to conversation

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
#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]>
#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.

#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

#2 lore raw
In reply to Jeff Garzik
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.

#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

#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

#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

#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

#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

#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

#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

#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

#13 lore raw

Re: s2io: patch disposition

In reply to Ravinandan Arakali
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

author
#12 lore raw

RE: s2io: patch disposition

In reply to Jeff Garzik

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

#14 lore raw

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

author
#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