From: ebiederm@xmission•com (Eric W. Biederman)
To: "Nicolas de Pesloüan" <nicolas.2p.debian@gmail•com>
Cc: Jiri Pirko <jpirko@redhat•com>, Jesse Gross <jesse@kernel•org>,
netdev@vger•kernel.org, davem@davemloft•net,
shemminger@linux-foundation•org, kaber@trash•net,
fubar@us•ibm.com, eric.dumazet@gmail•com, andy@greyhouse•net,
xiaosuo@gmail•com
Subject: Re: [patch net-next-2.6] net: vlan: make non-hw-accel rx path similar to hw-accel
Date: Mon, 04 Apr 2011 12:51:51 -0700 [thread overview]
Message-ID: <m1d3l1eqw8.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <4D9A1530.1010102@gmail.com> ("Nicolas de Pesloüan"'s message of "Mon, 04 Apr 2011 21:00:00 +0200")
Nicolas de Pesloüan <nicolas.2p.debian@gmail•com> writes:
> Le 04/04/2011 09:14, Jiri Pirko a écrit :
>> Mon, Apr 04, 2011 at 08:54:40AM CEST, nicolas.2p.debian@gmail•com wrote:
>>> Le 03/04/2011 22:38, Jesse Gross a écrit :
> <snip>
>>>> It would be nice to merge all of this together. One complication is
>>>> the interaction of bridging and vlan on the same device. Some people
>>>> want to have a bridge for each vlan and a bridge for untagged packets.
>>>> On older kernels with vlan accelerated hardware this was possible
>>>> because vlan devices would get packets before bridging and on current
>>>> kernels it is possible with ebtables rules. If we use rx_handler for
>>>> both I believe we would need to extend it some to allow multiple
>>>> handlers.
>>>
>>> I totally agree.
>>
>> I do not. The reason I do vlan_untag early is so actually emulates
>> hw acceleration. The reason is to make rx path of hwaccel an
>> nonhwaccel similar. If you move vlan untag to rx_handler, this goal
>> wouldn't be achieved.
>
> Need to think more about that point.
>
>>> Remember that Jiri's original proposal (last summer) was to have
>>> several rx_handlers per net_device. I still think we need several of
>>> them, because the network stack need to be generic and allow for any
>>> complex stacking setup. The rx_handler framework may need to be
>>> enhanced for that, but I think it is the right tool to do all those
>>> per net_device specific features.
>>>
>>>>> This would also cause protocol handlers to receive the untouched (tagged)
>>>>> frame, if no setup required the frame to be untagged, which I think is the
>>>>> right thing to do.
>>>>
>>>> At the very least we need to make sure that these packets are marked
>>>> as PACKET_OTHERHOST because protocol handlers don't pay attention to
>>>> the vlan field.
>>>
>>> Agreed.
>>>
>>>>>> @@ -3177,7 +3183,7 @@ ncls:
>>>>>> ret = deliver_skb(skb, pt_prev, orig_dev);
>>>>>> pt_prev = NULL;
>>>>>> }
>>>>>> - if (vlan_hwaccel_do_receive(&skb)) {
>>>>>> + if (vlan_do_receive(&skb)) {
>>>>>> ret = __netif_receive_skb(skb);
>>>>>> goto out;
>>>>>> } else if (unlikely(!skb))
>>>>>
>>>>> Why are you calling __netif_receive_skb here? Can't we simply goto
>>>>> another_round?
>>>>
>>>> This code (other than the name change) predates the
>>>> another_round/rx_handler changes.
>>>
>>> Yes, you are right. Let's keep this for a possible follow-up patch,
>>> to avoid skb reinjection when it is not strictly necessary.
>>
>> To do another round here was my attention do do in follow up patch (I'm
>> still figuring out how to move this effectively into rx_handlers)
>
> So you want to move vlan_do_receive into an rx_handler, but want untagging to
> stay hard-coded at the beginning of __netif_receive_skb. I don't think I
> understand the rational behind that.
__netif_receive_skb is actually late for untagging. eth_type_trans
would be better but not path of control into __netif_receive_skb
actually calls eth_type_trans.
Eric
next prev parent reply other threads:[~2011-04-04 19:51 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-02 10:26 [patch net-next-2.6] net: vlan: make non-hw-accel rx path similar to hw-accel Jiri Pirko
2011-04-02 15:55 ` Stephen Hemminger
2011-04-02 18:27 ` Jiri Pirko
2011-04-03 9:27 ` Nicolas de Pesloüan
2011-04-03 13:22 ` Jiri Pirko
2011-04-03 15:23 ` Nicolas de Pesloüan
2011-04-03 20:38 ` Jesse Gross
2011-04-04 6:54 ` Nicolas de Pesloüan
2011-04-04 7:14 ` Jiri Pirko
2011-04-04 19:00 ` Nicolas de Pesloüan
2011-04-04 19:51 ` Eric W. Biederman [this message]
2011-04-04 20:29 ` Jiri Pirko
2011-04-04 20:47 ` Nicolas de Pesloüan
2011-04-04 20:50 ` Jesse Gross
2011-04-04 21:04 ` Nicolas de Pesloüan
2011-04-05 7:25 ` Jiri Pirko
2011-04-05 7:26 ` Jiri Pirko
2011-04-04 20:30 ` Jiri Pirko
2011-04-04 20:51 ` Nicolas de Pesloüan
2011-04-05 7:19 ` Jiri Pirko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m1d3l1eqw8.fsf@fess.ebiederm.org \
--to=ebiederm@xmission$(echo .)com \
--cc=andy@greyhouse$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=fubar@us$(echo .)ibm.com \
--cc=jesse@kernel$(echo .)org \
--cc=jpirko@redhat$(echo .)com \
--cc=kaber@trash$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nicolas.2p.debian@gmail$(echo .)com \
--cc=shemminger@linux-foundation$(echo .)org \
--cc=xiaosuo@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox