From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f178.google.com (mail-pf0-f178.google.com [209.85.192.178]) by dpdk.org (Postfix) with ESMTP id 3FF6B68D1 for ; Tue, 6 Sep 2016 17:32:04 +0200 (CEST) Received: by mail-pf0-f178.google.com with SMTP id w87so17555363pfk.2 for ; Tue, 06 Sep 2016 08:32:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Rw5S106gVarqIzNif/VyYxsIht287b0cdzzk7qJ3eiA=; b=K+WGcg/Q+PuOr2j/L5LJwAV8ztDcgBFwIZBTGBillGcVsOrJtWGgYik6RakrlCOJN0 m4AbH1F0SGOhsXtN9e0TM3ufPdvSVW7DlNTSEVPAhbuG95c3ttC2rJqDQ9ATbSmtOQx2 a88GpascHv3RNCG2cCq58VpCg2ZtwNElrniqg+0mQ5TrfnAI/gePPJKaQ6juQDmkS68F MUR+8p/FssQSiR/XE3EAjWXXoUq8Xb9xmF6gx6gQ/tgbllJQc+l6azaOwV2cv4bKF+VX 9R7BoB5Wf4FXn9ahZYBuIrERAlyKLa1SzdVTJexjd4gJaFWgFCdcL88CvokQqsfQrOYM PMNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Rw5S106gVarqIzNif/VyYxsIht287b0cdzzk7qJ3eiA=; b=YVO5sxRAAxow0VYgwDHYiOUID/FyiQAFPGvfR6XQFlorhJgozCuGQppWXG/5c+KnJ4 Q659ZxgMyqn+VkeaDVRI6rGChWqfeJk4HWRsNs4KipROQX1/wH0VxIPQNGzpak284Ie8 F9ZI5vvWtSy6otuiD/v6ruTY6rhiOs9XXQ136Qohe6ou6i2xJw2vrTl7A/8g+mF0lAcS ywFcTUb2kf7eb2av0cDwuDcBN8hZ80Koe5/o0t8+uoeyvs+dXCe/yzdY1xZrEWKHWYy2 GSqaZsFdLT0CUq8ZrD36Uh634R1a1RLqoMNMKH/+CWlI2UuGRHm9da8UcFzuk3wfLbtw oMXw== X-Gm-Message-State: AE9vXwOTWu4C0LTANu7XJmfReun75g3nemPDOV5/ZJy5X9+FNFtRIVsjxN9vfqbcf+Ls4w== X-Received: by 10.98.66.74 with SMTP id p71mr46455504pfa.143.1473175923582; Tue, 06 Sep 2016 08:32:03 -0700 (PDT) Received: from xeon-e3 (static-50-53-69-251.bvtn.or.frontiernet.net. [50.53.69.251]) by smtp.gmail.com with ESMTPSA id o80sm41936219pfa.67.2016.09.06.08.32.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 06 Sep 2016 08:32:03 -0700 (PDT) Date: Tue, 6 Sep 2016 08:32:13 -0700 From: Stephen Hemminger To: "Pierre Pfister (ppfister)" Cc: Maxime Coquelin , Yuanhan Liu , "dev@dpdk.org" Message-ID: <20160906083213.6d2cc3bd@xeon-e3> In-Reply-To: <27D7470D-6FC7-4922-92A4-341CAD203689@cisco.com> References: <516F65D3-4706-4CC5-916B-6ECD29CBE177@cisco.com> <20160905022028.GF30752@yliu-dev.sh.intel.com> <6EFF45F1-172E-4470-B4D7-AED90474DE50@cisco.com> <1dfca8cf-e5ef-1040-8a40-d617ed03e5c0@redhat.com> <20160905140824.1f0c80a2@xeon-e3> <27D7470D-6FC7-4922-92A4-341CAD203689@cisco.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v2] virtio: enable indirect descriptors feature X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Sep 2016 15:32:04 -0000 On Tue, 6 Sep 2016 06:49:26 +0000 "Pierre Pfister (ppfister)" wrote: > > Le 5 sept. 2016 =C3=A0 23:08, Stephen Hemminger a =C3=A9crit : > >=20 > > On Mon, 5 Sep 2016 16:24:13 +0200 > > Maxime Coquelin wrote: > > =20 > >> Thanks Pierre for sending the fix. > >>=20 > >> Minor comments below: > >>=20 > >> On 09/05/2016 08:52 AM, Pierre Pfister (ppfister) wrote: =20 > >>> Indirect descriptors support was disabled by commit 4a92b67151be11, > >>> presumably by accident as it was correctly supported. > >>>=20 > >>> This patch simply adds VIRTIO_RING_F_INDIRECT_DESC back to > >>> the supported features bit mask, hence enabling the use of > >>> indirect descriptors when the feature is negociated with the > >>> device. > >>> =20 > >>=20 > >> You should add the below line: > >> Fixes: 4a92b671 ("virtio: clarify feature bit handling") > >>=20 > >> Also, maybe we should consider add stable@dpdk.org in cc:, > >> because the regression was introduced before v16.07 final tag. > >> But the problem is that all the final validation has been done > >> without this feature enabled, and it impact quite a few lines of > >> code in Virtio PMD. > >>=20 > >> Other than that, you can add: > >> Reviewed-by: Maxime Coquelin =20 > >=20 > > The patch is correct, but it doesn't fix a regression. > >=20 > > The original virtio DPDK did not support INDIRECT descriptors at all. > > The original code in virtio_negotiate features was the inverse of what = it is now. > > Read carefully, the values in mask were the bits that were rejected dur= ing > > guest negotiation at the time. > >=20 > > static void > > virtio_negotiate_features(struct virtio_hw *hw) > > { > > - uint32_t host_features, mask; > > - > > - /* checksum offload not implemented */ > > - mask =3D VIRTIO_NET_F_CSUM | VIRTIO_NET_F_GUEST_CSUM; > > - > > - /* TSO and LRO are only available when their corresponding > > - * checksum offload feature is also negotiated. > > - */ > > - mask |=3D VIRTIO_NET_F_HOST_TSO4 | VIRTIO_NET_F_HOST_TSO6 | VIRTIO_NE= T_F_HOST_ECN; > > - mask |=3D VIRTIO_NET_F_GUEST_TSO4 | VIRTIO_NET_F_GUEST_TSO6 | VIRTIO_= NET_F_GUEST_ECN; > > - mask |=3D VTNET_LRO_FEATURES; > > - > > - /* not negotiating INDIRECT descriptor table support */ > > - mask |=3D VIRTIO_RING_F_INDIRECT_DESC; > > + uint32_t host_features; > >=20 > > /* Prepare guest_features: feature that driver wants to support */ > > - hw->guest_features =3D VTNET_FEATURES & ~mask; > > + hw->guest_features =3D VIRTIO_PMD_GUEST_FEATURES; > > PMD_INIT_LOG(DEBUG, "guest_features before negotiate =3D %x", > > hw->guest_features); > >=20 > > Therefore INDIRECT descriptors were always disabled! Don't blame any c= ommit. > > Use of indirect descriptors by DPDK did not happen until a later change= . =20 >=20 > Oops. My bad, sorry. I will remove the blame from the commit message. > It is strange though that 6dc5de3a6aef adds indirect buffer support but d= oes not > provide a way to enable it. I agree that indirect descriptors should be negotiated. Probably the lack of the bit is left over from testing the three ways to send.