* [PATCH][BNX2X] added register coments
From: Eliezer Tamir @ 2007-11-01 9:28 UTC (permalink / raw)
To: davem@davemloft.net, netdev@vger.kernel.org; +Cc: Michael Chan, jeff@garzik.org
Dave,
Here is the version with added register comments.
Please consider applying.
ftp link:
ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-0.4.12-with-reg-remarks.txt
gzipped:
ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-0.4.12-with-reg-remarks.txt.gz
(I will also post each individual file for review as a reply to this
email)
Thanks,
Eliezer
^ permalink raw reply
* Re: [PATCH] net: docbook fixes for netif_ functions
From: David Miller @ 2007-11-01 9:22 UTC (permalink / raw)
To: rdunlap; +Cc: shemminger, netdev
In-Reply-To: <20071031153620.01cf5eeb.rdunlap@xenotime.net>
From: Randy Dunlap <rdunlap@xenotime.net>
Date: Wed, 31 Oct 2007 15:36:20 -0700
> > + * return values (usually ignored).
> > + * NET_RX_SUCCESS (no congestion)
> > + * NET_RX_DROP (packet was dropped)
>
> For the 3 lines above, how about:
>
> * Return values (usually ignored):
> * NET_RX_SUCCESS: no congestion
> * NET_RX_DROP: packet was dropped
>
>
> only because they come out of kernel-doc badly, munged together like so:
>
> return values (usually ignored). NET_RX_SUCCESS (no congestion)
> NET_RX_DROP (packet was dropped)
I've applied Stephen's patch with this minor correction
added.
^ permalink raw reply
* Re: [PATCH 5/5] Hide the net_ns kmem cache
From: David Miller @ 2007-11-01 7:47 UTC (permalink / raw)
To: xemul; +Cc: netdev, ebiederm, devel
In-Reply-To: <4728D867.8050207@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 22:32:55 +0300
> This cache is only required to create new namespaces,
> but we won't have them in CONFIG_NET_NS=n case.
>
> Hide it under the appropriate ifdef.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 4/5] Mark the setup_net as __net_init
From: David Miller @ 2007-11-01 7:46 UTC (permalink / raw)
To: xemul; +Cc: netdev, ebiederm, devel
In-Reply-To: <4728D822.30906@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 22:31:46 +0300
> The setup_net is called for the init net namespace
> only (int the CONFIG_NET_NS=n of course) from the __init
> function, so mark it as __net_init to disappear with the
> caller after the boot.
>
> Yet again, in the perfect world this has to be under
> #ifdef CONFIG_NET_NS, but it isn't guaranteed that every
> subsystem is registered *after* the init_net_ns is set
> up. After we are sure, that we don't start registering
> them before the init net setup, we'll be able to move
> this code under the ifdef.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 3/5] Hide the dead code in the net_namespace.c
From: David Miller @ 2007-11-01 7:45 UTC (permalink / raw)
To: xemul; +Cc: netdev, ebiederm, devel
In-Reply-To: <4728D773.5090402@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 22:28:51 +0300
> The namespace creation/destruction code is never called
> if the CONFIG_NET_NS is n, so it's OK to move it under
> appropriate ifdef.
>
> The copy_net_ns() in the "n" case checks for flags and
> returns -EINVAL when new net ns is requested. In a perfect
> world this stub must be in net_namespace.h, but this
> function need to know the CLONE_NEWNET value and thus
> requires sched.h. On the other hand this header is to be
> injected into almost every .c file in the networking code,
> and making all this code depend on the sched.h is a
> suicidal attempt.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 6/8] Make the sk_clone() lighter
From: Pavel Emelyanov @ 2007-11-01 8:46 UTC (permalink / raw)
To: David Miller; +Cc: netdev, devel
In-Reply-To: <20071101.002606.172611674.davem@davemloft.net>
David Miller wrote:
> From: Pavel Emelyanov <xemul@openvz.org>
> Date: Wed, 31 Oct 2007 16:54:34 +0300
>
>> The sk_prot_alloc() already performs all the stuff needed by the
>> sk_clone(). Besides, the sk_prot_alloc() requires almost twice
>> less arguments than the sk_alloc() does, so call the sk_prot_alloc()
>> saving the stack a bit.
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>>
>> ---
>>
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index e7537e4..c032f48 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -976,8 +976,9 @@ void sk_free(struct sock *sk)
>>
>> struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
>> {
>> - struct sock *newsk = sk_alloc(sk->sk_net, sk->sk_family, priority, sk->sk_prot, 0);
>> -
>> + struct sock *newsk;
>> +
>> + newsk = sk_prot_alloc(sk->sk_prot, priority, sk->sk_family);
>> if (newsk != NULL) {
>> struct sk_filter *filter;
>>
>
> After we make this change, what will set up newsk->sk_net?
This will be done automatically in the sock_copy().
> That's part of what sk_alloc() was doing for us, and that's
> why we need to pass the extra argument.
>
No it wasn't doing it for us, because the sk_net assignment was
done inside the if (zero_it) branch, but zero_it is 0 in this case.
Thanks,
Pavel
^ permalink raw reply
* Re: [PATCH 2/5] Relax the reference counting of init_net_ns
From: David Miller @ 2007-11-01 7:43 UTC (permalink / raw)
To: xemul; +Cc: netdev, ebiederm, devel
In-Reply-To: <4728D69E.6030808@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 22:25:18 +0300
> When the CONFIG_NET_NS is n there's no need in refcounting
> the initial net namespace. So relax this code by making a
> stupid stubs for the "n" case.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 1/5][NETNS] Make the init/exit hooks checks outside the loop
From: David Miller @ 2007-11-01 7:43 UTC (permalink / raw)
To: xemul; +Cc: netdev, ebiederm, devel
In-Reply-To: <4728D637.5030106@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 22:23:35 +0300
> When the new pernet something (subsys, device or operations) is
> being registered, the init callback is to be called for each
> namespace, that currently exitst in the system. During the
> unregister, the same is to be done with the exit callback.
>
> However, not every pernet something has both calls, but the
> check for the appropriate pointer to be not NULL is performed
> inside the for_each_net() loop.
>
> This is (at least) strange, so tune this.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 8/8] Forget the zero_it argument of sk_alloc()
From: David Miller @ 2007-11-01 7:41 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <47288A34.7010207@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:59:16 +0300
> Finally, the zero_it argument can be completely removed from
> the callers and from the function prototype.
>
> Besides, fix the checkpatch.pl warnings about using the
> assignments inside if-s.
>
> This patch is rather big, and it is a part of the previous one.
> I splitted it wishing to make the patches more readable. Hope
> this particular split helped.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 7/8] Remove bogus zero_it argument from sk_alloc
From: David Miller @ 2007-11-01 7:38 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <4728897A.50607@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:56:10 +0300
> At this point nobody calls the sk_alloc(() with zero_it == 0,
> so remove unneeded checks from it.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 6/8] Make the sk_clone() lighter
From: David Miller @ 2007-11-01 7:38 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <4728891A.30305@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:54:34 +0300
> The sk_prot_alloc() already performs all the stuff needed by the
> sk_clone(). Besides, the sk_prot_alloc() requires almost twice
> less arguments than the sk_alloc() does, so call the sk_prot_alloc()
> saving the stack a bit.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
I reread this patch and now I understand why it's correct.
When zero_it argument is zero, as was the case here, all of
the sk->sk_net processing is not done.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 5/8] Move some core sock setup into sk_prot_alloc
From: David Miller @ 2007-11-01 7:36 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <47288877.10803@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:51:51 +0300
> The security_sk_alloc() and the module_get is a part of the
> object allocations - move it in the proper place.
>
> Note, that since we do not reset the newly allocated sock
> in the sk_alloc() (memset() is removed with the previous
> patch) we can safely do this.
>
> Also fix the error path in sk_prot_alloc() - release the security
> context if needed.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 4/8] Auto-zero the allocated sock object
From: David Miller @ 2007-11-01 7:35 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <472887CB.9050203@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:48:59 +0300
> We have a __GFP_ZERO flag that allocates a zeroed chunk of memory.
> Use it in the sk_alloc() and avoid a hand-made memset().
>
> This is a temporary patch that will help us in the nearest future :)
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 3/8] Cleanup the allocation/freeing of the sock object
From: David Miller @ 2007-11-01 7:34 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <47288754.70307@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:47:00 +0300
> The sock object is allocated either from the generic cache with
> the kmalloc, or from the proc->slab cache.
>
> Move this logic into an isolated set of helpers and make the
> sk_alloc/sk_free look a bit nicer.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 2/8] Move the get_net() from sock_copy()
From: David Miller @ 2007-11-01 7:32 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <472886C8.5010601@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:44:40 +0300
> The sock_copy() is supposed to just clone the socket. In a perfect
> world it has to be just memcpy, but we have to handle the security
> mark correctly. All the extra setup must be performed in sk_clone()
> call, so move the get_net() into more proper place.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 1/8] Move the sock_copy() from the header
From: David Miller @ 2007-11-01 7:30 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <47288629.3010200@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:42:01 +0300
> The sock_copy() call is not used outside the sock.c file,
> so just move it into a sock.c
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 6/8] Make the sk_clone() lighter
From: David Miller @ 2007-11-01 7:26 UTC (permalink / raw)
To: xemul; +Cc: netdev, devel
In-Reply-To: <4728891A.30305@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 31 Oct 2007 16:54:34 +0300
> The sk_prot_alloc() already performs all the stuff needed by the
> sk_clone(). Besides, the sk_prot_alloc() requires almost twice
> less arguments than the sk_alloc() does, so call the sk_prot_alloc()
> saving the stack a bit.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>
> ---
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index e7537e4..c032f48 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -976,8 +976,9 @@ void sk_free(struct sock *sk)
>
> struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
> {
> - struct sock *newsk = sk_alloc(sk->sk_net, sk->sk_family, priority, sk->sk_prot, 0);
> -
> + struct sock *newsk;
> +
> + newsk = sk_prot_alloc(sk->sk_prot, priority, sk->sk_family);
> if (newsk != NULL) {
> struct sk_filter *filter;
>
After we make this change, what will set up newsk->sk_net?
That's part of what sk_alloc() was doing for us, and that's
why we need to pass the extra argument.
^ permalink raw reply
* Re: [PATCH 0/5] Make nicer CONFIG_NET_NS=n case code
From: David Miller @ 2007-11-01 7:02 UTC (permalink / raw)
To: dada1; +Cc: ebiederm, xemul, netdev, devel
In-Reply-To: <47297933.7090609@cosmosbay.com>
From: Eric Dumazet <dada1@cosmosbay.com>
Date: Thu, 01 Nov 2007 07:58:59 +0100
> I agree with you that with current state, this atomic_inc/atomic_dec_and_test
> wont come in profiles unless a trivial bench is writen
>
> for(;;){close(socket(....));}
Just add one packet send and one packet receive in there and you have
a transaction workload. It's really not that unrealistic.
^ permalink raw reply
* Re: [PATCH 0/5] Make nicer CONFIG_NET_NS=n case code
From: Eric Dumazet @ 2007-11-01 6:58 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Pavel Emelyanov, David Miller, Linux Netdev List, devel
In-Reply-To: <m1wst2hjo5.fsf@ebiederm.dsl.xmission.com>
Eric W. Biederman a écrit :
> Eric Dumazet <dada1@cosmosbay.com> writes:
>
>> Eric W. Biederman a écrit :
>>> Eric Dumazet <dada1@cosmosbay.com> writes:
>>>
>>>
>>>> Definitly wanted here. Thank you.
>>>> One more refcounting on each socket creation/deletion was expensive.
>>> Really? Have you actually measured that? If the overhead is
>>> measurable and expensive we may want to look at per cpu counters or
>>> something like that. So far I don't have any numbers that say any
>>> of the network namespace work inherently has any overhead.
>> It seems that on some old opterons (two 246 for example),
>> "if (atomic_dec_and_test(&net->count))" is rather expensive yes :(
>
> I won't argue that atomic_dec_and_test is costly. My gut feel is that
> socket creation/destruction is sufficiently rare that such a test
> would be lost in the noise. Doing anything more sophisticated is
> likely to be less readable, and unless we can measure some overhead
> my preference right now is to keep the code stupid and simple. Which
> usually has a good icache footprint.
I agree with you that with current state, this atomic_inc/atomic_dec_and_test
wont come in profiles unless a trivial bench is writen
for(;;){close(socket(....));}
If David or another dev can eliminate the atomic inc/dec on device refcount
cost for each packet traveling, the socket creation/destruction would
certainly raise.
Other contention points is the mnt_count (yet another refcount) in "struct
vfsmount", a truly useless refcount as I never had (and nobody had) to
un-mount sock_mnt :)
^ permalink raw reply
* Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
From: David Miller @ 2007-11-01 4:50 UTC (permalink / raw)
To: shemminger; +Cc: greearb, djohnson+linux-kernel, linux-kernel, netdev, bguo
In-Reply-To: <47292A99.5070805@linux-foundation.org>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Wed, 31 Oct 2007 18:23:37 -0700
> The code in AF_PACKET should fix the skb before passing to user
> space so that there is no difference between accel and non-accel
> hardware. Internal choices shouldn't leak to user space. Ditto,
> the receive checksum offload should be fixed up as well.
The hardware has stripped the VLAN header completely and has not
provided it to us at all.
In my opinion trying to cobble one up by hand using the known TAG is
worse than not providing a VLAN header at all.
^ permalink raw reply
* [PATCH] - e1000e/ethtool.c - convert macros to functions
From: Joe Perches @ 2007-11-01 3:29 UTC (permalink / raw)
To: Kok, Auke; +Cc: e1000-devel, netdev, Jeff Garzik
In-Reply-To: <1193877263.11020.48.camel@localhost>
Add functions for reg_pattern_test and reg_set_and check
Changed macros to use these functions
Compiled x86, untested
Size decreased ~2K
old:
$ size drivers/net/e1000e/ethtool.o
text data bss dec hex filename
14461 0 0 14461 387d drivers/net/e1000e/ethtool.o
new:
$ size drivers/net/e1000e/ethtool.o
text data bss dec hex filename
12498 0 0 12498 30d2 drivers/net/e1000e/ethtool.o
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/e1000e/ethtool.c | 78 +++++++++++++++++++++++++----------------
1 files changed, 47 insertions(+), 31 deletions(-)
diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 6a39784..225db17 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -691,41 +691,57 @@ err_setup:
return err;
}
-#define REG_PATTERN_TEST(R, M, W) REG_PATTERN_TEST_ARRAY(R, 0, M, W)
-#define REG_PATTERN_TEST_ARRAY(reg, offset, mask, writeable) \
-{ \
- u32 _pat; \
- u32 _value; \
- u32 _test[] = {0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF}; \
- for (_pat = 0; _pat < ARRAY_SIZE(_test); _pat++) { \
- E1000_WRITE_REG_ARRAY(hw, reg, offset, \
- (_test[_pat] & writeable)); \
- _value = E1000_READ_REG_ARRAY(hw, reg, offset); \
- if (_value != (_test[_pat] & writeable & mask)) { \
- ndev_err(netdev, "pattern test reg %04X " \
- "failed: got 0x%08X expected 0x%08X\n", \
- reg + offset, \
- value, (_test[_pat] & writeable & mask)); \
- *data = reg; \
- return 1; \
- } \
- } \
+bool reg_pattern_test_array(struct e1000_adapter *adapter, u64 *data,
+ int reg, int offset, u32 mask, u32 write)
+{
+ int i;
+ u32 read;
+ static const u32 test[] =
+ {0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+ for (i = 0; i < ARRAY_SIZE(test); i++) {
+ E1000_WRITE_REG_ARRAY(&adapter->hw, reg, offset,
+ (test[i] & write));
+ read = E1000_READ_REG_ARRAY(&adapter->hw, reg, offset);
+ if (read != (test[i] & write & mask)) {
+ ndev_err(adapter->netdev, "pattern test reg %04X "
+ "failed: got 0x%08X expected 0x%08X\n",
+ reg + offset,
+ read, (test[i] & write & mask));
+ *data = reg;
+ return true;
+ }
+ }
+ return false;
}
-#define REG_SET_AND_CHECK(R, M, W) \
-{ \
- u32 _value; \
- __ew32(hw, R, W & M); \
- _value = __er32(hw, R); \
- if ((W & M) != (_value & M)) { \
- ndev_err(netdev, "set/check reg %04X test failed: " \
- "got 0x%08X expected 0x%08X\n", R, (_value & M), \
- (W & M)); \
- *data = R; \
- return 1; \
- } \
+#define REG_PATTERN_TEST(R, M, W) \
+ if (reg_pattern_test_array(adapter, data, R, 0, M, W)) \
+ return 1;
+
+#define REG_PATTERN_TEST_ARRAY(R, offset, M, W) \
+ if (reg_pattern_test_array(adapter, data, R, offset, M, W)) \
+ return 1;
+
+static bool reg_set_and_check(struct e1000_adapter *adapter, u64 *data,
+ int reg, u32 mask, u32 write)
+{
+ u32 read;
+ __ew32(&adapter->hw, reg, write & mask);
+ read = __er32(&adapter->hw, reg);
+ if ((write & mask) != (read & mask)) {
+ ndev_err(adapter->netdev, "set/check reg %04X test failed: "
+ "got 0x%08X expected 0x%08X\n", reg, (read & mask),
+ (write & mask));
+ *data = reg;
+ return true;
+ }
+ return false;
}
+#define REG_SET_AND_CHECK(R, M, W) \
+ if (reg_set_and_check(adapter, data, R, M, W)) \
+ return 1;
+
static int e1000_reg_test(struct e1000_adapter *adapter, u64 *data)
{
struct e1000_hw *hw = &adapter->hw;
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
^ permalink raw reply related
* RE: [PATCH] ucc_geth: add support for netpoll
From: Li Yang-r58472 @ 2007-11-01 2:33 UTC (permalink / raw)
To: cbou; +Cc: netdev, linux-kernel, linuxppc-dev
In-Reply-To: <20071031215903.GA1287@zarina>
> -----Original Message-----
> From: Anton Vorontsov [mailto:cbou@mail.ru]
> Sent: Thursday, November 01, 2007 5:59 AM
> To: Li Yang-r58472
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] ucc_geth: add support for netpoll
>
> On Mon, Oct 29, 2007 at 03:17:44PM +0300, Anton Vorontsov wrote:
> [...]
> > > Oops. The original patch happened to hit the Junk mail box. :(
> >
> > That one as well? http://lkml.org/lkml/2007/10/11/128
> >
> > > I think
> > > the patch is good to merge after the cosmetic change. I
> can do it
> > > in next pull request to Jeff.
> >
> > Ok, great. Thanks.
>
> I'm wondering if you missed that email again. Maybe your mail
> client/server doing weird things with emails from @ru.mvista.com?
No. I have explicitly add you to the whitelist. :) Please be patient,
isn't this patch a new feature which can only be integrated in the merge
window? Thanks.
- Leo
^ permalink raw reply
* Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
From: Ben Greear @ 2007-11-01 1:31 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Dave Johnson, linux-kernel, netdev, Bin Guo
In-Reply-To: <47292A99.5070805@linux-foundation.org>
Stephen Hemminger wrote:
>
> The code in AF_PACKET should fix the skb before passing to user space
> so that there is
> no difference between accel and non-accel hardware. Internal choices
> shouldn't
> leak to user space. Ditto, the receive checksum offload should be
> fixed up as well.
Ok, I guess that will fix the sniffing issues and any user-space
bridging type applications.
Currently, VLAN devices offer the ability to 'reorder' the header and
explicitly remove the VLAN
header. I assume we keep this feature and have the AF_PACKET logic
check the device
flags to see if it should insert the VLAN header for hw-accel vlans?
Either way, if we sniff the underlying device, we should always get the
VLAN header.
What about drivers and filtering VLANs? It seems there is still a
difference between software
vlans and hw-accel in this case.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
From: Stephen Hemminger @ 2007-11-01 1:23 UTC (permalink / raw)
To: Ben Greear; +Cc: Dave Johnson, linux-kernel, netdev, Bin Guo
In-Reply-To: <4729269A.6090606@candelatech.com>
Ben Greear wrote:
> Stephen Hemminger wrote:
>> On Wed, 31 Oct 2007 14:43:51 -0400
>> Dave Johnson <djohnson+linux-kernel@sw.starentnetworks.com> wrote:
>>
>>
>>> Depending on the network driver, I'm seeing different behavior if
>>> a .1q packet is received to an PF_PACKET, SOCK_RAW, ETH_P_ALL socket.
>>>
>>>
>>> On devices what do not use NETIF_F_HW_VLAN_RX, the packet socket gets
>>> the complete packet with vlan tag included as the driver simply calls
>>> netif_receive_skb() or equivilant. packet_rcv() then gets the whole
>>> thing vlan tag included and sends this through the socket.
>>>
>>> vlan_skb_recv() also gets these all and will drop them because there
>>> are no vlans configured.
>>>
>>>
>>
>> The VLAN acceleration grabs and hides the tag. It is a design flaw
>> that should be fixed, feel free to post a patch.
>>
> There may be several ways to 'fix' this. Perhaps it would be worth
> discussing what
> we want the end result to be at least?
>
> Should we always pass the vlan header up to raw sockets as part of the
> data payload?
>
> Or, maybe pass it in an auxiliary message such as how timestamps may
> be passed?
>
> The first option seems cleaner, but maybe there are performance
> problems with this
> approach?
>
> We should also define what a NIC should do with VLANs it doesn't
> explicitly know
> about. I think it should pass them up the stack with VLAN tag
> intact, but again, perhaps
> there are reasons not to do that?
>
> DaveM did the HW Accel for VLANs if I remember correctly...perhaps he
> has some input?
The code in AF_PACKET should fix the skb before passing to user space so
that there is
no difference between accel and non-accel hardware. Internal choices
shouldn't
leak to user space. Ditto, the receive checksum offload should be fixed
up as well.
^ permalink raw reply
* Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
From: David Miller @ 2007-11-01 1:10 UTC (permalink / raw)
To: greearb; +Cc: shemminger, djohnson+linux-kernel, linux-kernel, netdev, bguo
In-Reply-To: <4729269A.6090606@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Date: Wed, 31 Oct 2007 18:06:34 -0700
> DaveM did the HW Accel for VLANs if I remember correctly...perhaps he
> has some input?
Not really, I'm busy and also not motivated to work on this, someone
else will need to.
^ 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