From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
dev@dpdk.org, thomas@monjalon.net, tiwei.bie@intel.com,
zhihong.wang@intel.com, jfreiman@redhat.com
Cc: stable@dpdk.org
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] vhost: fix packed ring defines declaration
Date: Thu, 22 Nov 2018 18:23:23 +0000 [thread overview]
Message-ID: <be131e16-1fe8-a02b-8299-76f3a17a07d1@intel.com> (raw)
In-Reply-To: <20181122170922.15007-1-maxime.coquelin@redhat.com>
On 11/22/2018 5:09 PM, Maxime Coquelin wrote:
> The packed ring defines were declared only if kernel
> header does not declare them.
> The problem is that they are not applied in upstream kernel,
> and some changes in the names have been required.
>
> This patch declares the defines unconditionally, which
> fixes potential build issues.
+1 to address possible build issues.
>
> Fixes: 297b1e7350f6 ("vhost: add virtio packed virtqueue defines")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> lib/librte_vhost/vhost.h | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> index 760f42192..5218f1b12 100644
> --- a/lib/librte_vhost/vhost.h
> +++ b/lib/librte_vhost/vhost.h
> @@ -219,13 +219,6 @@ struct vhost_msg {
>
> #define VIRTIO_F_RING_PACKED 34
>
> -#define VRING_DESC_F_NEXT 1
> -#define VRING_DESC_F_WRITE 2
> -#define VRING_DESC_F_INDIRECT 4
Why these are not re-defined below? Not used?
> -
> -#define VRING_DESC_F_AVAIL (1ULL << 7)
> -#define VRING_DESC_F_USED (1ULL << 15)
> -
> struct vring_packed_desc {
> uint64_t addr;
> uint32_t len;
> @@ -233,16 +226,23 @@ struct vring_packed_desc {
> uint16_t flags;
> };
>
> -#define VRING_EVENT_F_ENABLE 0x0
> -#define VRING_EVENT_F_DISABLE 0x1
> -#define VRING_EVENT_F_DESC 0x2
> -
> struct vring_packed_desc_event {
> uint16_t off_wrap;
> uint16_t flags;
> };
> #endif
>
> +/*
> + * Declare below packed ring defines unconditionally
> + * as Kernel header might use different names.
> + */
> +#define VRING_DESC_F_AVAIL (1ULL << 7)
> +#define VRING_DESC_F_USED (1ULL << 15)
> +
> +#define VRING_EVENT_F_ENABLE 0x0
> +#define VRING_EVENT_F_DISABLE 0x1
> +#define VRING_EVENT_F_DESC 0x2
What if Linux changes mind and uses old names again, build will fail again. If
related part in Linux is not released yet, what do you think being on safe side
and adding these defines with "#ifndef" wrap?
> +
> /*
> * Available and used descs are in same order
> */
>
next prev parent reply other threads:[~2018-11-22 18:23 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-22 17:09 [dpdk-stable] " Maxime Coquelin
2018-11-22 18:23 ` Ferruh Yigit [this message]
2018-11-22 18:51 ` [dpdk-stable] [dpdk-dev] " Maxime Coquelin
2018-11-22 19:20 ` Ferruh Yigit
2018-11-22 19:50 ` Maxime Coquelin
2018-11-22 22:04 ` Ferruh Yigit
2018-11-22 22:14 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=be131e16-1fe8-a02b-8299-76f3a17a07d1@intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=jfreiman@redhat.com \
--cc=maxime.coquelin@redhat.com \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
--cc=tiwei.bie@intel.com \
--cc=zhihong.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).