From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qc0-f182.google.com (mail-qc0-f182.google.com [209.85.216.182]) by dpdk.org (Postfix) with ESMTP id F14985A83 for ; Wed, 10 Jun 2015 02:48:32 +0200 (CEST) Received: by qcnj1 with SMTP id j1so12201117qcn.0 for ; Tue, 09 Jun 2015 17:48:32 -0700 (PDT) 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:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=LTmgS5v3XPYv6vXhocnwX+TZx0TIlx5bB8wewMKaUmo=; b=lnxenQkygn4ECauOXxqtIbyoaXzTmt1/xwGxoznfAELPt14cvibUXo06b45a8i+ZdV H92R4U5jBD7lPMt1Qto+3NmS1L5H4ERWekOhbD31jCaqt0zRNdSaN4GRvxvBnAhwN5L5 aG8PKe9SRSM1CR3G70zKmC17jyKNssG3ClXmym94DRY81Qrp3i9qa5/Q3bR/01/padn1 O2FjuHARxkrAhfs2f0Y5sQKgrZ+AWA1ozFSIRfRr82eBw8LHw8T/ioL5mpapGLXeIR2F GZ72YPbC2n55AKlztWZz9sVEXi8SE8bTHs/AAw19NYl+NUt704BBP/Mv5BHGw7S+eAS4 CqJw== X-Gm-Message-State: ALoCoQkz86j1AVlS6xFUkN5U2Mg4vpETdVF20TTuf4CTbda1HITg4oEnmJgzVICXV25m0lF+mnDS X-Received: by 10.140.133.199 with SMTP id 190mr513386qhf.17.1433897312543; Tue, 09 Jun 2015 17:48:32 -0700 (PDT) Received: from urahara (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by mx.google.com with ESMTPSA id 78sm3441489qhb.10.2015.06.09.17.48.31 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 09 Jun 2015 17:48:32 -0700 (PDT) Date: Tue, 9 Jun 2015 17:48:34 -0700 From: Stephen Hemminger To: dev@dpdk.org Message-ID: <20150609174834.3c6006ac@urahara> In-Reply-To: <1429111219-8789-6-git-send-email-stephen@networkplumber.org> References: <1429111219-8789-1-git-send-email-stephen@networkplumber.org> <1429111219-8789-6-git-send-email-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 5/5] virtio: clarify feature bit handling 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: Wed, 10 Jun 2015 00:48:33 -0000 On Wed, 15 Apr 2015 08:20:19 -0700 Stephen Hemminger wrote: > Change the features from bit mask to bit number. This allows the > DPDK driver to use the definitions from Linux[1]. This makes doing > enhancements to DPDK driver easier when new feature bits are added. > > More importantly, get rid of the confusing feature bit initialization > code. Remove the double negative code in the feature masking. > Instead just have a new define with the list of feature bits implemented. > > Signed-off-by: Stephen Hemminger > --- > lib/librte_pmd_virtio/virtio_ethdev.c | 17 +------ > lib/librte_pmd_virtio/virtio_ethdev.h | 27 ++++------ > lib/librte_pmd_virtio/virtio_pci.h | 96 ++++++++++++++++++----------------- > lib/librte_pmd_virtio/virtqueue.h | 8 +-- > 4 files changed, 61 insertions(+), 87 deletions(-) > > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c > index db0232e..349b73b 100644 > --- a/lib/librte_pmd_virtio/virtio_ethdev.c > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c > @@ -807,23 +807,10 @@ virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) > static void > virtio_negotiate_features(struct virtio_hw *hw) > { > - uint32_t host_features, mask; > - > - /* checksum offload not implemented */ > - mask = 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 |= VIRTIO_NET_F_HOST_TSO4 | VIRTIO_NET_F_HOST_TSO6 | VIRTIO_NET_F_HOST_ECN; > - mask |= VIRTIO_NET_F_GUEST_TSO4 | VIRTIO_NET_F_GUEST_TSO6 | VIRTIO_NET_F_GUEST_ECN; > - mask |= VTNET_LRO_FEATURES; > - > - /* not negotiating INDIRECT descriptor table support */ > - mask |= VIRTIO_RING_F_INDIRECT_DESC; > + uint32_t host_features; > > /* Prepare guest_features: feature that driver wants to support */ > - hw->guest_features = VTNET_FEATURES & ~mask; > + hw->guest_features = VIRTIO_PMD_GUEST_FEATURES; > PMD_INIT_LOG(DEBUG, "guest_features before negotiate = %x", > hw->guest_features); > > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.h b/lib/librte_pmd_virtio/virtio_ethdev.h > index e6d4533..df2cb7d 100644 > --- a/lib/librte_pmd_virtio/virtio_ethdev.h > +++ b/lib/librte_pmd_virtio/virtio_ethdev.h > @@ -56,24 +56,15 @@ > #define VIRTIO_MAX_RX_PKTLEN 9728 > > /* Features desired/implemented by this driver. */ > -#define VTNET_FEATURES \ > - (VIRTIO_NET_F_MAC | \ > - VIRTIO_NET_F_STATUS | \ > - VIRTIO_NET_F_MQ | \ > - VIRTIO_NET_F_CTRL_MAC_ADDR | \ > - VIRTIO_NET_F_CTRL_VQ | \ > - VIRTIO_NET_F_CTRL_RX | \ > - VIRTIO_NET_F_CTRL_VLAN | \ > - VIRTIO_NET_F_CSUM | \ > - VIRTIO_NET_F_HOST_TSO4 | \ > - VIRTIO_NET_F_HOST_TSO6 | \ > - VIRTIO_NET_F_HOST_ECN | \ > - VIRTIO_NET_F_GUEST_CSUM | \ > - VIRTIO_NET_F_GUEST_TSO4 | \ > - VIRTIO_NET_F_GUEST_TSO6 | \ > - VIRTIO_NET_F_GUEST_ECN | \ > - VIRTIO_NET_F_MRG_RXBUF | \ > - VIRTIO_RING_F_INDIRECT_DESC) > +#define VIRTIO_PMD_GUEST_FEATURES \ > + (1u << VIRTIO_NET_F_MAC | \ > + 1u << VIRTIO_NET_F_STATUS | \ > + 1u << VIRTIO_NET_F_MQ | \ > + 1u << VIRTIO_NET_F_CTRL_MAC_ADDR | \ > + 1u << VIRTIO_NET_F_CTRL_VQ | \ > + 1u << VIRTIO_NET_F_CTRL_RX | \ > + 1u << VIRTIO_NET_F_CTRL_VLAN | \ > + 1u << VIRTIO_NET_F_MRG_RXBUF) > > /* > * CQ function prototype > diff --git a/lib/librte_pmd_virtio/virtio_pci.h b/lib/librte_pmd_virtio/virtio_pci.h > index 64d9c34..47f722a 100644 > --- a/lib/librte_pmd_virtio/virtio_pci.h > +++ b/lib/librte_pmd_virtio/virtio_pci.h > @@ -96,26 +96,6 @@ struct virtqueue; > #define VIRTIO_CONFIG_STATUS_FAILED 0x80 > > /* > - * Generate interrupt when the virtqueue ring is > - * completely used, even if we've suppressed them. > - */ > -#define VIRTIO_F_NOTIFY_ON_EMPTY (1 << 24) > - > -/* > - * The guest should never negotiate this feature; it > - * is used to detect faulty drivers. > - */ > -#define VIRTIO_F_BAD_FEATURE (1 << 30) > - > -/* > - * Some VirtIO feature bits (currently bits 28 through 31) are > - * reserved for the transport being used (eg. virtio_ring), the > - * rest are per-device feature bits. > - */ > -#define VIRTIO_TRANSPORT_F_START 28 > -#define VIRTIO_TRANSPORT_F_END 32 > - > -/* > * Each virtqueue indirect descriptor list must be physically contiguous. > * To allow us to malloc(9) each list individually, limit the number > * supported to what will fit in one page. With 4KB pages, this is a limit > @@ -128,33 +108,55 @@ struct virtqueue; > #define VIRTIO_MAX_INDIRECT ((int) (PAGE_SIZE / 16)) > > /* The feature bitmap for virtio net */ > -#define VIRTIO_NET_F_CSUM 0x00001 /* Host handles pkts w/ partial csum */ > -#define VIRTIO_NET_F_GUEST_CSUM 0x00002 /* Guest handles pkts w/ partial csum*/ > -#define VIRTIO_NET_F_MAC 0x00020 /* Host has given MAC address. */ > -#define VIRTIO_NET_F_GSO 0x00040 /* Host handles pkts w/ any GSO type */ > -#define VIRTIO_NET_F_GUEST_TSO4 0x00080 /* Guest can handle TSOv4 in. */ > -#define VIRTIO_NET_F_GUEST_TSO6 0x00100 /* Guest can handle TSOv6 in. */ > -#define VIRTIO_NET_F_GUEST_ECN 0x00200 /* Guest can handle TSO[6] w/ ECN in.*/ > -#define VIRTIO_NET_F_GUEST_UFO 0x00400 /* Guest can handle UFO in. */ > -#define VIRTIO_NET_F_HOST_TSO4 0x00800 /* Host can handle TSOv4 in. */ > -#define VIRTIO_NET_F_HOST_TSO6 0x01000 /* Host can handle TSOv6 in. */ > -#define VIRTIO_NET_F_HOST_ECN 0x02000 /* Host can handle TSO[6] w/ ECN in. */ > -#define VIRTIO_NET_F_HOST_UFO 0x04000 /* Host can handle UFO in. */ > -#define VIRTIO_NET_F_MRG_RXBUF 0x08000 /* Host can merge receive buffers. */ > -#define VIRTIO_NET_F_STATUS 0x10000 /* virtio_net_config.status available*/ > -#define VIRTIO_NET_F_CTRL_VQ 0x20000 /* Control channel available */ > -#define VIRTIO_NET_F_CTRL_RX 0x40000 /* Control channel RX mode support */ > -#define VIRTIO_NET_F_CTRL_VLAN 0x80000 /* Control channel VLAN filtering */ > -#define VIRTIO_NET_F_CTRL_RX_EXTRA 0x100000 /* Extra RX mode control support */ > -#define VIRTIO_RING_F_INDIRECT_DESC 0x10000000 /* Support for indirect buffer descriptors. */ > -/* The guest publishes the used index for which it expects an interrupt > - * at the end of the avail ring. Host should ignore the avail->flags field. > - * The host publishes the avail index for which it expects a kick > - * at the end of the used ring. Guest should ignore the used->flags field. > +#define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */ > +#define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */ > +#define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */ > +#define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */ > +#define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */ > +#define VIRTIO_NET_F_GUEST_ECN 9 /* Guest can handle TSO[6] w/ ECN in. */ > +#define VIRTIO_NET_F_GUEST_UFO 10 /* Guest can handle UFO in. */ > +#define VIRTIO_NET_F_HOST_TSO4 11 /* Host can handle TSOv4 in. */ > +#define VIRTIO_NET_F_HOST_TSO6 12 /* Host can handle TSOv6 in. */ > +#define VIRTIO_NET_F_HOST_ECN 13 /* Host can handle TSO[6] w/ ECN in. */ > +#define VIRTIO_NET_F_HOST_UFO 14 /* Host can handle UFO in. */ > +#define VIRTIO_NET_F_MRG_RXBUF 15 /* Host can merge receive buffers. */ > +#define VIRTIO_NET_F_STATUS 16 /* virtio_net_config.status available */ > +#define VIRTIO_NET_F_CTRL_VQ 17 /* Control channel available */ > +#define VIRTIO_NET_F_CTRL_RX 18 /* Control channel RX mode support */ > +#define VIRTIO_NET_F_CTRL_VLAN 19 /* Control channel VLAN filtering */ > +#define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support */ > +#define VIRTIO_NET_F_GUEST_ANNOUNCE 21 /* Guest can announce device on the > + * network */ > +#define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow > + * Steering */ > +#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > + > +/* Do we get callbacks when the ring is completely used, even if we've > + * suppressed them? */ > +#define VIRTIO_F_NOTIFY_ON_EMPTY 24 > + > +/* Can the device handle any descriptor layout? */ > +#define VIRTIO_F_ANY_LAYOUT 27 > + > +/* We support indirect buffer descriptors */ > +#define VIRTIO_RING_F_INDIRECT_DESC 28 > + > +/* > + * Some VirtIO feature bits (currently bits 28 through 31) are > + * reserved for the transport being used (eg. virtio_ring), the > + * rest are per-device feature bits. > */ > -#define VIRTIO_RING_F_EVENT_IDX 0x20000000 > +#define VIRTIO_TRANSPORT_F_START 28 > +#define VIRTIO_TRANSPORT_F_END 32 > + > +/* The Guest publishes the used index for which it expects an interrupt > + * at the end of the avail ring. Host should ignore the avail->flags field. */ > +/* The Host publishes the avail index for which it expects a kick > + * at the end of the used ring. Guest should ignore the used->flags field. */ > +#define VIRTIO_RING_F_EVENT_IDX 29 > > -#define VIRTIO_NET_S_LINK_UP 1 /* Link is up */ > +#define VIRTIO_NET_S_LINK_UP 1 /* Link is up */ > +#define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */ > > /* > * Maximum number of virtqueues per device. > @@ -243,9 +245,9 @@ outl_p(unsigned int data, unsigned int port) > outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))) > > static inline int > -vtpci_with_feature(struct virtio_hw *hw, uint32_t feature) > +vtpci_with_feature(struct virtio_hw *hw, uint32_t bit) > { > - return (hw->guest_features & feature) != 0; > + return (hw->guest_features & (1u << bit)) != 0; > } > > /* > diff --git a/lib/librte_pmd_virtio/virtqueue.h b/lib/librte_pmd_virtio/virtqueue.h > index 41dda50..5509173 100644 > --- a/lib/librte_pmd_virtio/virtqueue.h > +++ b/lib/librte_pmd_virtio/virtqueue.h > @@ -201,18 +201,12 @@ struct virtqueue { > }; > > /* If multiqueue is provided by host, then we suppport it. */ > -#ifndef VIRTIO_NET_F_MQ > -/* Device supports Receive Flow Steering */ > -#define VIRTIO_NET_F_MQ 0x400000 > #define VIRTIO_NET_CTRL_MQ 4 > #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET 0 > #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1 > #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000 > -#endif > -#ifndef VIRTIO_NET_F_CTRL_MAC_ADDR > -#define VIRTIO_NET_F_CTRL_MAC_ADDR 0x800000 > + > #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1 > -#endif > > /** > * This is the first element of the scatter-gather list. If you don't Why are all these virtio patches stuck in the DPDK dead zone. It makes me frustrated that no update to virtio has been done for 2.1