Re: [PATCH v5 08/17] fm10k: add RX/TX single queue start/stop function

[David Marchand <david.marchand-pdR9zngts4EAvxtiuMwx3w@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