Re: [dpdk-dev] [PATCH 03/18] fm10k: Add empty fm10k files

[David Marchand <david.marchand@6wind.com>]

On Fri, Jan 30, 2015 at 6:07 AM, Chen Jing D(Mark) <jing.d.chen@intel.com>
wrote:

> From: Jeff Shaw <jeffrey.b.shaw@intel.com>
>
> Define macros and basic data structure.
> Define rte_log wrapper functions.
>

This comment applies to the logs macro (and the rest of the patchset).
- don't use a build option for logs to be displayed, especially if these
are init messages or error messages that prevent the pmd from working
- you can remove this "Use RTE_LOG directly to make sure this error is
seen." in fm10k_rx_queue_setup if you use a "init" macro that is not under
a build option
- don't use \n in logs, only one is enough

Please, check the cleanup work that has been done in other Intel pmd (for
example, ixgbe).

I would really prefer we have consistent logs across dpdk.


-- 
David Marchand



>
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  lib/librte_pmd_fm10k/Makefile     |   96 ++++++++++++++++
>  lib/librte_pmd_fm10k/fm10k.h      |  224
> +++++++++++++++++++++++++++++++++++++
>  lib/librte_pmd_fm10k/fm10k_logs.h |   66 +++++++++++
>  3 files changed, 386 insertions(+), 0 deletions(-)
>  create mode 100644 lib/librte_pmd_fm10k/Makefile
>  create mode 100644 lib/librte_pmd_fm10k/fm10k.h
>  create mode 100644 lib/librte_pmd_fm10k/fm10k_ethdev.c
>  create mode 100644 lib/librte_pmd_fm10k/fm10k_logs.h
>  create mode 100644 lib/librte_pmd_fm10k/fm10k_rxtx.c
>
> diff --git a/lib/librte_pmd_fm10k/Makefile b/lib/librte_pmd_fm10k/Makefile
> new file mode 100644
> index 0000000..3d76387
> --- /dev/null
> +++ b/lib/librte_pmd_fm10k/Makefile
> @@ -0,0 +1,96 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2013-2014 Intel Corporation. All rights reserved.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +#     * Redistributions of source code must retain the above copyright
> +#       notice, this list of conditions and the following disclaimer.
> +#     * Redistributions in binary form must reproduce the above copyright
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#     * Neither the name of Intel Corporation nor the names of its
> +#       contributors may be used to endorse or promote products derived
> +#       from this software without specific prior written permission.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_fm10k.a
> +
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +
> +ifeq ($(CC), icc)
> +#
> +# CFLAGS for icc
> +#
> +CFLAGS_BASE_DRIVER = -wd174 -wd593 -wd869 -wd981 -wd2259
> +
> +else ifeq ($(CC), clang)
> +#
> +## CFLAGS for clang
> +#
> +CFLAGS_BASE_DRIVER = -Wno-unused-parameter -Wno-unused-value
> +CFLAGS_BASE_DRIVER += -Wno-strict-aliasing -Wno-format-extra-args
> +CFLAGS_BASE_DRIVER += -Wno-unused-variable -Wno-unused-but-set-variable
> +CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
> +
> +else
> +#
> +# CFLAGS for gcc
> +#
> +ifneq ($(shell test $(GCC_MAJOR_VERSION) -le 4 -a $(GCC_MINOR_VERSION)
> -le 3 && echo 1), 1)
> +CFLAGS     += -Wno-deprecated
> +endif
> +CFLAGS_BASE_DRIVER = -Wno-unused-parameter -Wno-unused-value
> +CFLAGS_BASE_DRIVER += -Wno-strict-aliasing -Wno-format-extra-args
> +CFLAGS_BASE_DRIVER += -Wno-unused-variable -Wno-unused-but-set-variable
> +CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
> +endif
> +
> +#
> +# Add extra flags for base driver source files to disable warnings in them
> +#
> +BASE_DRIVER_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard
> $(RTE_SDK)/lib/librte_pmd_fm10k/SHARED/*.c)))
> +$(foreach obj, $(BASE_DRIVER_OBJS), $(eval
> CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER)))
> +
> +VPATH += $(RTE_SDK)/lib/librte_pmd_fm10k/SHARED
> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_ethdev.c
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_rxtx.c
> +
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_pf.c
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_tlv.c
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_common.c
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_mbx.c
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_vf.c
> +SRCS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k_api.c
> +
> +# this lib depends upon:
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_eal lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_mempool
> lib/librte_mbuf
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_net lib/librte_malloc
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_pmd_fm10k/fm10k.h b/lib/librte_pmd_fm10k/fm10k.h
> new file mode 100644
> index 0000000..9b2d3da
> --- /dev/null
> +++ b/lib/librte_pmd_fm10k/fm10k.h
> @@ -0,0 +1,224 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2013-2014 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _FM10K_H_
> +#define _FM10K_H_
> +
> +#include <stdint.h>
> +#include <rte_mbuf.h>
> +#include <rte_mempool.h>
> +#include <rte_malloc.h>
> +#include <rte_spinlock.h>
> +#include "fm10k_logs.h"
> +#include "SHARED/fm10k_type.h"
> +
> +/* descriptor ring base addresses must be aligned to the following */
> +#define FM10K_ALIGN_RX_DESC  128
> +#define FM10K_ALIGN_TX_DESC  128
> +
> +/* The maximum packet size that FM10K supports */
> +#define FM10K_MAX_PKT_SIZE  (15 * 1024)
> +
> +/* Minimum size of RX buffer FM10K supported */
> +#define FM10K_MIN_RX_BUF_SIZE  256
> +
> +/* The maximum of SRIOV VFs per port supported */
> +#define FM10K_MAX_VF_NUM    64
> +
> +/* number of descriptors must be a multiple of the following */
> +#define FM10K_MULT_RX_DESC  FM10K_REQ_RX_DESCRIPTOR_MULTIPLE
> +#define FM10K_MULT_TX_DESC  FM10K_REQ_TX_DESCRIPTOR_MULTIPLE
> +
> +/* maximum size of descriptor rings */
> +#define FM10K_MAX_RX_RING_SZ  (512 * 1024)
> +#define FM10K_MAX_TX_RING_SZ  (512 * 1024)
> +
> +/* minimum and maximum number of descriptors in a ring */
> +#define FM10K_MIN_RX_DESC  32
> +#define FM10K_MIN_TX_DESC  32
> +#define FM10K_MAX_RX_DESC  (FM10K_MAX_RX_RING_SZ / sizeof(union
> fm10k_rx_desc))
> +#define FM10K_MAX_TX_DESC  (FM10K_MAX_TX_RING_SZ / sizeof(struct
> fm10k_tx_desc))
> +
> +/*
> + * byte aligment for HW RX data buffer
> + * Datasheet requires RX buffer addresses shall either be 512-byte
> aligned or
> + * be 8-byte aligned but without crossing host memory pages (4KB alignment
> + * boundaries). Satisfy first option.
> + */
> +#define FM10K_RX_DATABUF_ALIGN 512
> +
> +/*
> + * threshold default, min, max, and divisor constraints
> + * the configured values must satisfy the following:
> + *   MIN <= value <= MAX
> + *   DIV % value == 0
> + */
> +#define FM10K_RX_FREE_THRESH_DEFAULT(rxq)  32
> +#define FM10K_RX_FREE_THRESH_MIN(rxq)      1
> +#define FM10K_RX_FREE_THRESH_MAX(rxq)      ((rxq)->nb_desc - 1)
> +#define FM10K_RX_FREE_THRESH_DIV(rxq)      ((rxq)->nb_desc)
> +
> +#define FM10K_TX_FREE_THRESH_DEFAULT(txq)  32
> +#define FM10K_TX_FREE_THRESH_MIN(txq)      1
> +#define FM10K_TX_FREE_THRESH_MAX(txq)      ((txq)->nb_desc - 3)
> +#define FM10K_TX_FREE_THRESH_DIV(txq)      0
> +
> +#define FM10K_DEFAULT_RX_PTHRESH      8
> +#define FM10K_DEFAULT_RX_HTHRESH      8
> +#define FM10K_DEFAULT_RX_WTHRESH      0
> +
> +#define FM10K_DEFAULT_TX_PTHRESH      32
> +#define FM10K_DEFAULT_TX_HTHRESH      0
> +#define FM10K_DEFAULT_TX_WTHRESH      0
> +
> +#define FM10K_TX_RS_THRESH_DEFAULT(txq)    32
> +#define FM10K_TX_RS_THRESH_MIN(txq)        1
> +#define FM10K_TX_RS_THRESH_MAX(txq)        \
> +       RTE_MIN(((txq)->nb_desc - 2), (txq)->free_thresh)
> +#define FM10K_TX_RS_THRESH_DIV(txq)        ((txq)->nb_desc)
> +
> +#define FM10K_VLAN_TAG_SIZE 4
> +
> +struct fm10k_dev_info {
> +       volatile uint32_t enable;
> +       volatile uint32_t glort;
> +       /* Protect the mailbox to avoid race condition */
> +       rte_spinlock_t    mbx_lock;
> +};
> +
> +/*
> + * Structure to store private data for each driver instance.
> + */
> +struct fm10k_adapter {
> +       struct fm10k_hw             hw;
> +       struct fm10k_hw_stats       stats;
> +       struct fm10k_dev_info       info;
> +};
> +
> +#define FM10K_DEV_PRIVATE_TO_HW(adapter) \
> +       (&((struct fm10k_adapter *)adapter)->hw)
> +
> +#define FM10K_DEV_PRIVATE_TO_STATS(adapter) \
> +       (&((struct fm10k_adapter *)adapter)->stats)
> +
> +#define FM10K_DEV_PRIVATE_TO_INFO(adapter) \
> +       (&((struct fm10k_adapter *)adapter)->info)
> +
> +#define FM10K_DEV_PRIVATE_TO_MBXLOCK(adapter) \
> +       (&(((struct fm10k_adapter *)adapter)->info.mbx_lock))
> +
> +struct fm10k_rx_queue {
> +       struct rte_mempool *mp;
> +       struct rte_mbuf **sw_ring;
> +       volatile union fm10k_rx_desc *hw_ring;
> +       struct rte_mbuf *pkt_first_seg; /**< First segment of current
> packet. */
> +       struct rte_mbuf *pkt_last_seg;  /**< Last segment of current
> packet. */
> +       uint64_t hw_ring_phys_addr;
> +       uint16_t next_dd;
> +       uint16_t next_alloc;
> +       uint16_t next_trigger;
> +       uint16_t alloc_thresh;
> +       volatile uint32_t *tail_ptr;
> +       uint16_t nb_desc;
> +       uint16_t queue_id;
> +       uint8_t port_id;
> +       uint8_t drop_en;
> +       uint8_t rx_deferred_start; /** < don't start this queue in dev
> start. */
> +};
> +
> +/*
> + * a FIFO is used to track which descriptors have their RS bit set for Tx
> + * queues which are configured to allow multiple descriptors per packet
> + */
> +struct fifo {
> +       uint16_t *list;
> +       uint16_t *head;
> +       uint16_t *tail;
> +       uint16_t *endp;
> +};
> +
> +struct fm10k_tx_queue {
> +       struct rte_mbuf **sw_ring;
> +       struct fm10k_tx_desc *hw_ring;
> +       uint64_t hw_ring_phys_addr;
> +       struct fifo rs_tracker;
> +       uint16_t last_free;
> +       uint16_t next_free;
> +       uint16_t nb_free;
> +       uint16_t nb_used;
> +       uint16_t free_trigger;
> +       uint16_t free_thresh;
> +       uint16_t rs_thresh;
> +       volatile uint32_t *tail_ptr;
> +       uint16_t nb_desc;
> +       uint8_t port_id;
> +       uint8_t tx_deferred_start; /** < don't start this queue in dev
> start. */
> +       uint16_t queue_id;
> +};
> +
> +#define MBUF_DMA_ADDR(mb) \
> +       ((uint64_t) ((mb)->buf_physaddr + (mb)->data_off))
> +
> +/* enforce 512B alignment on default Rx DMA addresses */
> +#define MBUF_DMA_ADDR_DEFAULT(mb) \
> +       ((uint64_t) RTE_ALIGN(((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM),
> 512))
> +
> +static inline void fifo_reset(struct fifo *fifo, uint32_t len)
> +{
> +       fifo->head = fifo->tail = fifo->list;
> +       fifo->endp = fifo->list + len;
> +}
> +
> +static inline void fifo_insert(struct fifo *fifo, uint16_t val)
> +{
> +       *fifo->head = val;
> +       if (++fifo->head == fifo->endp)
> +               fifo->head = fifo->list;
> +}
> +
> +/* do not worry about list being empty since we only check it once we know
> + * we have used enough descriptors to set the RS bit at least once */
> +static inline uint16_t fifo_peek(struct fifo *fifo)
> +{
> +       return *fifo->tail;
> +}
> +
> +static inline uint16_t fifo_remove(struct fifo *fifo)
> +{
> +       uint16_t val;
> +       val = *fifo->tail;
> +       if (++fifo->tail == fifo->endp)
> +               fifo->tail = fifo->list;
> +       return val;
> +}
> +#endif
> diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> new file mode 100644
> index 0000000..e69de29
> diff --git a/lib/librte_pmd_fm10k/fm10k_logs.h
> b/lib/librte_pmd_fm10k/fm10k_logs.h
> new file mode 100644
> index 0000000..0b4cd24
> --- /dev/null
> +++ b/lib/librte_pmd_fm10k/fm10k_logs.h
> @@ -0,0 +1,66 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2013-2014 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _FM10K_LOGS_H_
> +#define _FM10K_LOGS_H_
> +
> +#include <rte_log.h>
> +
> +#ifdef RTE_LIBRTE_FM10K_DEBUG
> +#define PMD_LOG(level, fmt, args...) \
> +       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +#define PMD_FUNC_TRACE() PMD_LOG(DEBUG, " >>")
> +#else
> +#define PMD_LOG(level, fmt, args...) do { } while (0)
> +#define PMD_FUNC_TRACE() do { } while (0)
> +#endif
> +
> +#ifdef RTE_LIBRTE_FM10K_DEBUG_RX
> +#define PMD_LOG_RX(level, fmt, args...) \
> +       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +#define PMD_FUNC_TRACE_RX() PMD_LOG_RX(DEBUG, " >>")
> +#else
> +#define PMD_LOG_RX(level, fmt, args...) do { } while (0)
> +#define PMD_FUNC_TRACE_RX() do { } while (0)
> +#endif
> +
> +#ifdef RTE_LIBRTE_FM10K_DEBUG_TX
> +#define PMD_LOG_TX(level, fmt, args...) \
> +       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +#define PMD_FUNC_TRACE_TX() PMD_LOG_TX(DEBUG, " >>")
> +#else
> +#define PMD_LOG_TX(level, fmt, args...) do { } while (0)
> +#define PMD_FUNC_TRACE_TX() do { } while (0)
> +#endif
> +
> +#endif /* _FM10K_LOGS_H_ */
> diff --git a/lib/librte_pmd_fm10k/fm10k_rxtx.c
> b/lib/librte_pmd_fm10k/fm10k_rxtx.c
> new file mode 100644
> index 0000000..e69de29
> --
> 1.7.7.6
>
>