From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Marchand <david.marchand-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH v5 08/17] fm10k: add RX/TX single queue
	start/stop function
Date: Fri, 13 Feb 2015 12:31:16 +0100
Message-ID: <CALwxeUshpFKc--RpnqML4HVdp_q2j70OHPgnbVHwm6pMU-4+AQ@mail.gmail.com>
References: <1423618298-2933-2-git-send-email-jing.d.chen@intel.com>
 <1423815597-17819-1-git-send-email-jing.d.chen@intel.com>
 <1423815597-17819-9-git-send-email-jing.d.chen@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Cc: "dev-VfR2kkLFssw@public.gmane.org" <dev-VfR2kkLFssw@public.gmane.org>
To: "Chen Jing D(Mark)" <jing.d.chen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <1423815597-17819-9-git-send-email-jing.d.chen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

Hello,

On Fri, Feb 13, 2015 at 9:19 AM, Chen Jing D(Mark) <jing.d.chen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
wrote:

[snip]

+/*
> + * Verify Rx packet buffer alignment is valid.
> + *
> + * Hardware requires specific alignment for Rx packet buffers. At
> + * least one of the following two conditions must be satisfied.
> + *  1. Address is 512B aligned
> + *  2. Address is 8B aligned and buffer does not cross 4K boundary.
> + *
> + * Return 1 if buffer alignment satisfies at least one condition,
> + * otherwise return 0.
> + *
> + * Note: Alignment is checked by the driver when the Rx queue is reset. It
> + *       is assumed that if an entire descriptor ring can be filled with
> + *       buffers containing valid alignment, then all buffers in that
> mempool
> + *       have valid address alignment. It is the responsibility of the
> user
> + *       to ensure all buffers have valid alignment, as it is the user who
> + *       creates the mempool.
> + * Note: It is assumed the buffer needs only to store a maximum size
> Ethernet
> + *       frame.
> + */
> +static inline int
> +fm10k_addr_alignment_valid(struct rte_mbuf *mb)
> +{
> +       uint64_t addr = MBUF_DMA_ADDR_DEFAULT(mb);
> +       uint64_t boundary1, boundary2;
> +
> +       /* 512B aligned? */
> +       if (RTE_ALIGN(addr, 512) == addr)
> +               return 1;
> +
> +       /* 8B aligned, and max Ethernet frame would not cross a 4KB
> boundary? */
> +       if (RTE_ALIGN(addr, 8) == addr) {
> +               boundary1 = RTE_ALIGN_FLOOR(addr, 4096);
> +               boundary2 = RTE_ALIGN_FLOOR(addr +
> ETHER_MAX_VLAN_FRAME_LEN,
> +                                               4096);
> +               if (boundary1 == boundary2)
> +                       return 1;
> +       }
> +
> +       /* use RTE_LOG directly to make sure this error is seen */
> +       RTE_LOG(ERR, PMD, "%s(): Error: Invalid buffer alignment\n",
> __func__);
> +
> +       return 0;
> +}
>

Same comment as before, do not directly use RTE_LOG.
This is init stuff, you have a PMD_INIT_LOG macro.

By the way, I need to dig deeper into this, but I can see multiple patches
ensuring buffer alignment.
Do we really need to validate this alignment here, if we already validated
this constraint at the mempool level ?


-- 
David Marchand