From patchwork Mon Oct 25 23:23:55 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?ISO-8859-2?Q?Micha=B3_Miros=B3aw?= X-Patchwork-Id: 69149 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 5DC94B6F10 for ; Tue, 26 Oct 2010 10:24:10 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932180Ab0JYXX5 (ORCPT ); Mon, 25 Oct 2010 19:23:57 -0400 Received: from mail-qy0-f181.google.com ([209.85.216.181]:34745 "EHLO mail-qy0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932156Ab0JYXX4 convert rfc822-to-8bit (ORCPT ); Mon, 25 Oct 2010 19:23:56 -0400 Received: by qyk10 with SMTP id 10so2273684qyk.19 for ; Mon, 25 Oct 2010 16:23:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=U2Oa9QxQqtRXg+MPdfCy82Mr+oiVPrWioH1Hc7VIZ6g=; b=vE8X50UlyypbZqtUouCirwHjjDIRDfIkA0B8F0LdYlJt7nYsswxJUzyl8jIOr/nTTQ R8/3DhVvzkP8eg/NB3FqrqV6/GYkrPuW10YnjLCK7++lCPbpRwVxSMOGwQwLgCNSy1BW uujydSyXt+is7VA5DABbEU4aMGysc9M7VhPIg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=tzAgL5/V1ZW9kz3nH8mLs5pgskI1/knCDl/XHWIGbUFVRRnyPUYuQEO2MEz4taiOfU 2ZH/DgJeYyl9qpV/222WR8ApUTf3HseFB0B6VX2l1I9nF9m1TxlFKdZu6TcpfHFvyE2y 6KKcmzobHbg3FAhjKxLMOTkZnE5iBKyRgAeKM= MIME-Version: 1.0 Received: by 10.229.183.20 with SMTP id ce20mr242396qcb.203.1288049035258; Mon, 25 Oct 2010 16:23:55 -0700 (PDT) Received: by 10.229.182.71 with HTTP; Mon, 25 Oct 2010 16:23:55 -0700 (PDT) In-Reply-To: <4CC5FE81.4020606@intel.com> References: <1287618974-4714-1-git-send-email-jesse@nicira.com> <1287618974-4714-11-git-send-email-jesse@nicira.com> <1288029009.1808.1.camel@pjaxe> <4CC5FE81.4020606@intel.com> Date: Tue, 26 Oct 2010 01:23:55 +0200 Message-ID: Subject: Re: [PATCH v2 10/14] ixgbe: Update ixgbe to use new vlan accleration. From: =?ISO-8859-2?Q?Micha=B3_Miros=B3aw?= To: John Fastabend Cc: "Waskiewicz Jr, Peter P" , Jesse Gross , David Miller , "netdev@vger.kernel.org" , "Tantilov, Emil S" , "Kirsher, Jeffrey T" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org W dniu 26 października 2010 00:02 użytkownik John Fastabend napisał: > On 10/25/2010 2:40 PM, Michał Mirosław wrote: >> W dniu 25 października 2010 19:50 użytkownik Peter P Waskiewicz Jr >> napisał: >>> On Fri, 2010-10-22 at 06:24 -0700, Michał Mirosław wrote: >>>> 2010/10/21 Jesse Gross : >>>>> Make the ixgbe driver use the new vlan accleration model. >>>> [...] >>>>> --- a/drivers/net/ixgbe/ixgbe_main.c >>>>> +++ b/drivers/net/ixgbe/ixgbe_main.c >>>>> @@ -954,17 +954,13 @@ static void ixgbe_receive_skb(struct ixgbe_q_vector *q_vector, >>>>>        bool is_vlan = (status & IXGBE_RXD_STAT_VP); >>>>>        u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan); >>>>> >>>>> -       if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL)) { >>>>> -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK)) >>>>> -                       vlan_gro_receive(napi, adapter->vlgrp, tag, skb); >>>>> -               else >>>>> -                       napi_gro_receive(napi, skb); >>>>> -       } else { >>>>> -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK)) >>>>> -                       vlan_hwaccel_rx(skb, adapter->vlgrp, tag); >>>>> -               else >>>>> -                       netif_rx(skb); >>>>> -       } >>>>> +       if (is_vlan && (tag & VLAN_VID_MASK)) >>>>> +               __vlan_hwaccel_put_tag(skb, tag); >>>> >>>> I know that this is carried over from the driver, but why tag == 0 is >>>> treated differently here? VID0 is somewhat special, as normally it >>>> means 802.1p packet, but i.e. in embedded world people are using it as >>>> normal VID. It would be nice to have this handled consistently in the >>>> VLAN core - deliver to base dev (tag stripped) if vlan 0 is not >>>> configured and to vlan dev if it is. >>> >>> ixgbe handles VLAN 0 differently because that's the tag that's used when >>> DCB is enabled, and no VLAN is configured.  We have to insert the 802.1p >>> tag for DCB to work, but the OS won't know about the 802.1q tag, and >>> ends up dropping the frame.  So we enable VLAN ID 0 in the HW and tell >>> it to strip the tag, so we can still pass the frame up the stack. >> >> So that's actually (part of) what I'm proposing but done at driver level. > I agree this should be handled outside the driver. Something like this should > do, Current code (Linus' tree, so with Jesse's VLAN rework applied) should work exactly as I proposed in my original mail - VLAN 0 is treated as 802.1p (stripped and delivered to original dev) unless vlan device with id 0 is configured. (Patch untested). Best Regards, Michał Mirosław --- ixgbe: Enable VLAN 0 Signed-off-by: Michał Mirosław if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL)) -- 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 diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c index f856312..1544368 100644 --- a/drivers/net/ixgbe/ixgbe_main.c +++ b/drivers/net/ixgbe/ixgbe_main.c @@ -956,7 +956,7 @@ static void ixgbe_receive_skb(struct ixgbe_q_vector *q_vector, bool is_vlan = (status & IXGBE_RXD_STAT_VP); u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan); - if (is_vlan && (tag & VLAN_VID_MASK)) + if (is_vlan) __vlan_hwaccel_put_tag(skb, tag);