From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753905AbbBMRer (ORCPT ); Fri, 13 Feb 2015 12:34:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58889 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753502AbbBMRep (ORCPT ); Fri, 13 Feb 2015 12:34:45 -0500 Message-ID: <54DE358B.90502@redhat.com> Date: Fri, 13 Feb 2015 12:34:03 -0500 From: Vlad Yasevich Reply-To: vyasevic@redhat.com Organization: Red Hat User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: "Skidmore, Donald C" , Hiroshi Shimamoto , "Kirsher, Jeffrey T" CC: Alexander Duyck , =?UTF-8?B?QmrDuHJuIE1vcms=?= , "e1000-devel@lists.sourceforge.net" , "netdev@vger.kernel.org" , "Choi, Sy Jong" , "linux-kernel@vger.kernel.org" , David Laight , Hayato Momma Subject: Re: [E1000-devel] [PATCH 1/3] ixgbe, ixgbevf: Add new mbox API to enable MC promiscuous mode References: <7F861DC0615E0C47A872E6F3C5FCDDBD05E1908D@BPXM14GP.gisp.nec.co.jp> <1423054641.2589.59.camel@jtkirshe-mobl> <7F861DC0615E0C47A872E6F3C5FCDDBD05E26E45@BPXM14GP.gisp.nec.co.jp> <1423470046.27854.7.camel@jtkirshe-mobl> <7F861DC0615E0C47A872E6F3C5FCDDBD05E29F43@BPXM14GP.gisp.nec.co.jp> <7F861DC0615E0C47A872E6F3C5FCDDBD05E2E9CE@BPXM14GP.gisp.nec.co.jp> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/13/2015 12:26 PM, Skidmore, Donald C wrote: > > >> -----Original Message----- >> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com] >> Sent: Thursday, February 12, 2015 8:45 PM >> To: Skidmore, Donald C; Kirsher, Jeffrey T >> Cc: Alexander Duyck; Bjørn Mork; e1000-devel@lists.sourceforge.net; >> netdev@vger.kernel.org; Choi, Sy Jong; linux-kernel@vger.kernel.org; David >> Laight; Hayato Momma >> Subject: RE: [E1000-devel] [PATCH 1/3] ixgbe, ixgbevf: Add new mbox API to >> enable MC promiscuous mode >> >>>>> -----Original Message----- >>>>> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com] >>>>> Sent: Monday, February 09, 2015 6:29 PM >>>>> To: Kirsher, Jeffrey T >>>>> Cc: Alexander Duyck; Skidmore, Donald C; Bjørn Mork; e1000- >>>>> devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy >>>>> Jong; linux- kernel@vger.kernel.org; David Laight; Hayato Momma >>>>> Subject: RE: [E1000-devel] [PATCH 1/3] ixgbe, ixgbevf: Add new >>>>> mbox API to enable MC promiscuous mode >>>>> >>>>>>>>> Can you please fix up your patches based on my tree: >>>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/que >>>>>>>>> ue.git >>>>>>>> >>>>>>>> Yes. I haven't noticed your tree. >>>>>>>> Will resend patches against it. >>>>>>>> >>>>>>> >>>>>>> I encountered an issue with your tree, the commit id is below. >>>>>>> >>>>>>> $ git log | head >>>>>>> commit e6f1649780f8f5a87299bf6af04453f93d1e3d5e >>>>>>> Author: Rasmus Villemoes >>>>>>> Date: Fri Jan 23 20:43:14 2015 -0800 >>>>>>> >>>>>>> ethernet: fm10k: Actually drop 4 bits >>>>>>> >>>>>>> The comment explains the intention, but vid has type u16. Before >> the >>>>>>> inner shift, it is promoted to int, which has plenty of space for all >>>>>>> vid's bits, so nothing is dropped. Use a simple mask instead. >>>>>>> >>>>>>> >>>>>>> I use the kernel from your tree in both host and guest. >>>>>>> >>>>>>> Assign an IPv6 for VF in guest. >>>>>>> # ip -6 addr add 2001:db8::18:1/64 dev ens0 >>>>>>> >>>>>>> Send ping packet from other server to the VM. >>>>>>> # ping6 2001:db8::18:1 -I eth0 >>>>>>> >>>>>>> The following message was shown. >>>>>>> ixgbevf 0000:00:08.0: partial checksum but l4 proto=3a! >>>>>>> >>>>>>> If I did the same operation in the host, I saw the same error >>>>>>> message in >>>>> host too. >>>>>>> ixgbe 0000:2d:00.0: partial checksum but l4 proto=3a! >>>>>>> >>>>>>> Do you have any idea about that? >>>>>> >>>>>> Ah, sorry about that, try this tree again: >>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/queue.git >>>>>> >>>>>> That patch was dropped for favor of a patch that Matthew Vick >>>>>> put together (and recently got pushed upstream). So my queue no >>>>>> longer has that patch in the queue, since it got dropped. >>>>> >>>>> I still see the same error, the head id is the below >>>>> >>>>> $ git log | head >>>>> commit a072afb0b45904022b76deef3b770ee9a93cb13a >>>>> Author: Nicholas Krause >>>>> Date: Mon Feb 9 00:27:00 2015 -0800 >>>>> >>>>> igb: Remove outdated fix me comment in the >>>>> function,gb_acquire_swfw_sync_i210 >>>>> >>>>> >>>>> thanks, >>>>> Hiroshi >>>> >>>> I'm having our validation see if they can recreate the same issue >>>> internally. When they get back to me I'll let you >>> know >>>> what we found. >>> >>> We did bisect, and the below looks the culprit; >>> >>> 32dce968dd987adfb0c00946d78dad9154f64759 is the first bad commit >>> commit 32dce968dd987adfb0c00946d78dad9154f64759 >>> Author: Vlad Yasevich >>> Date: Sat Jan 31 10:40:18 2015 -0500 >>> >>> ipv6: Allow for partial checksums on non-ufo packets >>> >>> Currntly, if we are not doing UFO on the packet, all UDP >>> packets will start with CHECKSUM_NONE and thus perform full >>> checksum computations in software even if device support >>> IPv6 checksum offloading. >>> >>> Let's start start with CHECKSUM_PARTIAL if the device >>> supports it and we are sending only a single packet at >>> or below mtu size. >>> >>> Signed-off-by: Vladislav Yasevich >>> Signed-off-by: David S. Miller >>> >>> :040000 040000 4437eaf7e944f5a6136ebf668a256fee688fda3d >> fade8da998d35c8da97a15f0556949ad371e5347 M net >> >> When I reverted the commit, the issue was solved. >> >> thanks, >> Hiroshi > > I believe the issue is that this patch (32dce968dd98 - ipv6: Allow for partial checksums on non-ufo packets) is that it now sets CHECKSUM_PARTIAL on all IPv6 packets including ICMPv6 ones. Our HW (82599) only supports checksum offload on TCP/UDP (NETIF_F_IPV6_CSUM) so we get hung up on the skb's protocol and the fact that it is CHECKSUM_PARTIAL. > > Another thing that confuses me is the feature test in this patch. It checks (rt->dst.dev->features & NETIF_F_V6_CSUM) but NETIF_F_V6_CSUM is a two bit field? > > #define NETIF_F_V6_CSUM (NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM) > > So the test would succeed if either bit was high, that doesn't seem right. I cc'd the author so maybe he could clue us in. This has been addressed by: commit bf250a1fa769f2eb8fc7a4e28b3b523e9cb67eef Author: Vlad Yasevich Date: Tue Feb 10 11:37:29 2015 -0500 ipv6: Partial checksum only UDP packets As far the 2 bit issue, GEN_CSUM (HW_SUM) and IPV6_CSUM can not coexist at the same time. See netdev_fix_features(). -vlad > > Thanks, > -Don Skidmore > >