From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id 71B0F5F17 for ; Mon, 3 Sep 2018 15:14:25 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id g33-v6so662256wrd.1 for ; Mon, 03 Sep 2018 06:14:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=czmGth6o53fQIMRzHa2I2HxXRjSQDzUCQHtEAa/8b68=; b=PJbUFE7dpLhq/Jvcw667L6Km3heWNUpqTXWGuXAwb4Pous9q5GncwA81C3cjGZbmuw TL6nqYwSc6PZaGDp5cf5BW1cZuXJGDgoonelyrIL8Bp3t5Nb3v5ZrNnYstyWDtsKfgDc IeiyJtGdLG0mRgHb/gExLNPdE6ArFWQARrNfNbYlAPcRlvGiQx3Kp0jBo94MNVI2KFSM HReJ5yQEVtbCyaI+DHV99F7PYtGcMAhb7M4iMaQja9YXtfVhdOVPUWZLVnaOv328VYTU Sduqu62xwF9Vyj0dtZmE+Eb7IoP5S4RLEez1IHNYi2+d/6aeWvumEjFusMkjwhxsghHg M44g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=czmGth6o53fQIMRzHa2I2HxXRjSQDzUCQHtEAa/8b68=; b=FDI7D7xndI/BqggCFi/5SXyF/x0lIDIsrSdDjlvxDFNP3G4BbL1oXxfGXRNodgBvNB Ir3Ykk6Au8VU2uqQ6EbCvSf1DZ7Wn71NpVd9z6a+u2+R+YF8dk5DD12VppBn1DJsIQVV aG+eGGXVCtJ1WJIJ7ILoXk4Aecvz/gw3yx8dVXXJsa5pmp7oRydZNw2Wj5ItuAuln+se coXaxG6+9w8gvXgWBs6g3jbekZlsayt5rbFF0IrewdxzMFab5UOsF83enSD2jHWPWcZ3 9gDDOYAIlMjBSYOVLpCvXJVwJGjGl8KGSjAyBLE2yfmnWt1gvyqnfnpxvGZg7L6v2MCU /9ag== X-Gm-Message-State: APzg51AN55wRe8YIhYqDnHDxzp3J7DT66qaBbcJ2nETKUdHDgiNpsFiY JUkBsCy1+z6oIJjaaagtETw= X-Google-Smtp-Source: ANB0VdaBSvdX9QgCxC7he7VTgA+Gqg6RXCbiyYnVigqFTNOsCAIxVPmc1RkvuJ3AC4aQVDckeV1goQ== X-Received: by 2002:adf:f410:: with SMTP id g16-v6mr19392674wro.256.1535980465048; Mon, 03 Sep 2018 06:14:25 -0700 (PDT) Received: from [10.156.47.67] ([137.221.143.78]) by smtp.gmail.com with ESMTPSA id l12-v6sm20055410wrv.29.2018.09.03.06.14.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 03 Sep 2018 06:14:24 -0700 (PDT) To: "Zhang, Qi Z" , "dev@dpdk.org" Cc: "Lu, Wenzhuo" , "Ananyev, Konstantin" , Robert Shearman References: <1535128501-31597-1-git-send-email-robertshearman@gmail.com> <039ED4275CED7440929022BC67E706115327F501@SHSMSX103.ccr.corp.intel.com> From: Robert Shearman Message-ID: <0ee303e8-0d63-eafb-ec77-b9e447183a76@gmail.com> Date: Mon, 3 Sep 2018 14:14:23 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <039ED4275CED7440929022BC67E706115327F501@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: Strip SR-IOV transparent VLANs in VF X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Sep 2018 13:14:25 -0000 Hi Qi, On 03/09/2018 12:45, Zhang, Qi Z wrote: > Hi Robert: > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of >> robertshearman@gmail.com >> Sent: Saturday, August 25, 2018 12:35 AM >> To: dev@dpdk.org >> Cc: Lu, Wenzhuo ; Ananyev, Konstantin >> ; Robert Shearman >> >> Subject: [dpdk-dev] [PATCH] net/ixgbe: Strip SR-IOV transparent VLANs in VF >> >> From: Robert Shearman >> >> SR-IOV VFs support "transparent" VLANs. Traffic from/to a VM associated with >> a VF has a VLAN tag inserted/stripped in a manner intended to be totally >> transparent to the VM. On a Linux hypervisor the vlan can be specified by "ip >> link set vf vlan ". >> The VM VF driver is not configured to use any VLAN and the VM should never >> see the transparent VLAN for that reason. However, in practice these VLAN >> headers are being received by the VM which discards the packets as that VLAN >> is unknown to it. The Linux kernel ixbge driver explicitly removes the VLAN in >> this case (presumably due to the hardware not being able to do this) but the >> DPDK driver does not. > > I'm not quite understand this part. > What does explicitly remove the VLAN means?, > DPDK also discard unmatched VLAN and strip vlan if vlan_strip is enabled what is the gap? > It will be better if you can give same examples Sure. Typical use case for this is a hypervisor where it is necessary to provide L2 access into the guests, but there are insufficient, and so the hypervisor is using the PF and VFs are assigned to guests. In order to avoid having to configure each guest to use the VLAN and to not send any untagged traffic it is desirable to use transparent VLANs. For example: Guest 1 = VLAN 10 Guest 2 = VLAN 20 ip link set eth0 vf 1 vlan 10 ip link set eth0 vf 2 vlan 20 Now this means that packets arriving tagged on the physical port should be delivered to the guest and arrive in the guest untagged. Similarly, packets transmitted untagged by the guest should gain a tag before they go out of the physical port. What you get when using the Linux VF ixgbe driver inside the VMs is exactly this since the driver knows that for this hardware the transparent stripping isn't done in hardware and is done inside the driver. What you get currently when using the DPDK VF ixgbe driver inside the VMs is that packets arrive tagged (e.g. with VLAN tag 10) and these are then dropped because the VM doesn't know about VLAN 10. Transparent VLAN insertion works currently with both Linux and DPDK VF drivers. > >> >> This patch mirrors the kernel driver behaviour by removing the VLAN on the VF >> side. This is done by checking the VLAN in the VFTA, where the hypervisor will >> have set the bit in the VFTA corresponding to the VLAN if transparent VLANs >> were being used for the VF. If the VLAN is set in the VFTA then it is known that >> it's a transparent VLAN case and so the VLAN is stripped from the mbuf. > > This is missing leading. > From your code, I only saw vlan flag in ol_flag is stripped, but not VLAN is stripped. > I think vlan is always be stripped if we enable vlan strip on queue. I think you're saying that the VLAN isn't removed if hardware RX VLAN stripping isn't configured. This is true, but might cost performance to cover this case too. If you're happy with that, then I can issue a V2 with that addressed. If you're suggesting that m->vlan_tci needs to be set to 0 when PKT_RX_VLAN is cleared from m->ol_flags, then I don't think that is necessary since my understanding is an application should only be looking at m->vlan_tci if m->ol_flags has PKT_RX_VLAN set. > >> To >> limit any potential performance impact on the PF data path, the RX path is split >> into PF and VF versions with the transparent VLAN stripping only done in the >> VF path. Measurements with our application show ~2% performance hit for >> the VF case and none for the PF case. >> > > ... > >> +/* >> + * Filter out unknown vlans resulting from use of transparent vlan. >> + * >> + * When a VF is configured to use transparent vlans then the VF can >> + * see this VLAN being set in the packet, meaning that the transparent >> + * property isn't preserved. Furthermore, when the VF is used in a >> + * guest VM then there's no way of knowing for sure that transparent >> + * VLAN is in use and what tag value has been configured. So work >> + * around this by removing the VLAN flag if the VF isn't interested in >> + * the VLAN tag. >> + */ >> +static inline void >> +ixgbevf_trans_vlan_sw_filter_hdr(struct rte_mbuf *m, >> + const struct ixgbe_vfta *vfta) >> +{ >> + if (m->ol_flags & PKT_RX_VLAN) { >> + uint16_t vlan = m->vlan_tci & 0xFFF; >> + >> + if (!ixgbe_vfta_is_vlan_set(vfta, vlan)) >> + m->ol_flags &= ~PKT_RX_VLAN; >> + } >> +} >> + > > Ideally all driver's behavior should be consistent with the same configure. > if "transparent vlan" looks like a general feature, it may not only bind to VF or even just ixgbevf. (what about i40evf?) > Otherwise, it should be handled in application , but not the driver. It's a general feature, but the implementation is specific to a driver. I believe that this is handled in hardware on i40e, but this is just based on the there being no special handling of this case in the RX path in the Linux i40e VF driver. Furthermore, transparent VLANs implemented in the application would just be called "VLANs" :-) More specifically, the application running in the guest cannot know what has been configured for the VF in the hypervisor in a driver-independent manner, or whether the hardware has in fact transparently removed the VLAN already (as may be the case for i40e). > > ... > >> + ixgbe_unknown_vlan_sw_filter_hdr(rx_pkts[pos + 3], vfta, rxq); > > Where is ixgbe_unknown_vlan_sw_filter_hdr be defined? I saw it is only be used in ixgbe_rxtx_vec_neon.c, so assume there will be a compile error on that platform? Good catch. I don't have the ability to compile for that platform, and missed the rename I did during development. Will fix in V2. Thanks, Rob