DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Alfredo Cardigliano <cardigliano@ntop.org>,
	John McNamara <john.mcnamara@intel.com>,
	Marko Kovacevic <marko.kovacevic@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 10/17] net/ionic: add basic port operations
Date: Mon, 2 Dec 2019 16:11:21 +0000
Message-ID: <662c5427-7417-7ae5-b40a-5a7175f58e84@intel.com> (raw)
In-Reply-To: <20191015082235.28639-11-cardigliano@ntop.org>

On 10/15/2019 9:22 AM, Alfredo Cardigliano wrote:
> Add support for port start/stop and handle basic features
> including mtu and link up/down.
> 
> Signed-off-by: Alfredo Cardigliano <cardigliano@ntop.org>
> Reviewed-by: Shannon Nelson <snelson@pensando.io>
> ---
>  doc/guides/nics/features/ionic.ini |   4 +
>  drivers/net/ionic/ionic.h          |   1 +
>  drivers/net/ionic/ionic_dev.h      |   3 +
>  drivers/net/ionic/ionic_ethdev.c   | 316 +++++++++++++++++++++++++++++
>  drivers/net/ionic/ionic_lif.c      | 268 ++++++++++++++++++++++++
>  drivers/net/ionic/ionic_lif.h      |   9 +
>  drivers/net/ionic/ionic_osdep.h    |   2 +
>  7 files changed, 603 insertions(+)

<...>

> @@ -18,6 +18,17 @@
>  
>  static int  eth_ionic_dev_init(struct rte_eth_dev *eth_dev, void *init_params);
>  static int  eth_ionic_dev_uninit(struct rte_eth_dev *eth_dev);
> +static int  ionic_dev_info_get(struct rte_eth_dev *eth_dev,
> +		struct rte_eth_dev_info *dev_info);
> +static int  ionic_dev_configure(struct rte_eth_dev *dev);
> +static int  ionic_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
> +static int  ionic_dev_start(struct rte_eth_dev *dev);
> +static void ionic_dev_stop(struct rte_eth_dev *dev);

Start and stop dev_ops should not return 'void' but 'int', I think you will see
this when rebase to latest code. Same for a few more dev_ops.

<...>

> +static int
> +ionic_dev_mtu_set(struct rte_eth_dev *eth_dev, uint16_t mtu)
> +{
> +	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
> +	/*
> +	 * Size = MTU + Ethernet header + VLAN + QinQ
> +	 * Also add ETHER_CRC_LEN if the adapter is able to keep CRC
> +	 */
> +	uint32_t frame_size = mtu + RTE_ETHER_HDR_LEN + 4 + 4;
> +	int err;
> +
> +	ionic_init_print_call();
> +
> +	/* Check that mtu is within the allowed range */
> +	if (mtu < IONIC_MIN_MTU || mtu > IONIC_MAX_MTU)
> +		return -EINVAL;

If you feed these values to dev_info min_mtu/max_mtu, this check will be done by
API level already.

> +
> +	err = ionic_lif_change_mtu(lif, mtu);
> +
> +	if (err)
> +		return err;
> +
> +	/* Update max frame size */
> +	eth_dev->data->dev_conf.rxmode.max_rx_pkt_len = frame_size;

'max_rx_pkt_len' is a user provided configuration value to limit the packet size
in jumbo frame case, and it shouldn't just updated to reflect the MTU.

<...>

> +static int
> +ionic_dev_start(struct rte_eth_dev *eth_dev)
> +{
> +	struct rte_eth_conf *dev_conf = &eth_dev->data->dev_conf;
> +	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
> +	struct ionic_adapter *adapter = lif->adapter;
> +	struct ionic_dev *idev = &adapter->idev;
> +	uint32_t allowed_speeds;
> +	int err;
> +
> +	ionic_init_print_call();
> +
> +	err = ionic_lif_start(lif);
> +
> +	if (err) {
> +		ionic_drv_print(ERR, "Cannot start LIF: %d", err);
> +		return err;
> +	}
> +
> +	if (eth_dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED) {
> +		uint32_t speed = ionic_parse_link_speeds(dev_conf->link_speeds);
> +
> +		if (speed)
> +			ionic_dev_cmd_port_speed(idev, speed);
> +	}
> +
> +	allowed_speeds =
> +		ETH_LINK_SPEED_FIXED |
> +		ETH_LINK_SPEED_10G |
> +		ETH_LINK_SPEED_25G |
> +		ETH_LINK_SPEED_40G |
> +		ETH_LINK_SPEED_50G |
> +		ETH_LINK_SPEED_100G;
> +
> +	if (dev_conf->link_speeds & ~allowed_speeds) {
> +		ionic_init_print(ERR, "Invalid link setting");

Should this check be done before 'ionic_lif_start()' call?

<...>

> +static void
> +ionic_dev_close(struct rte_eth_dev *eth_dev)
> +{
> +	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
> +	int err;
> +
> +	ionic_init_print_call();
> +
> +	err = ionic_lif_stop(lif);
> +
> +	if (err) {
> +		ionic_drv_print(ERR, "Cannot stop LIF: %d", err);
> +		return;
> +	}

'close()' dev_ops should free all device resources. A closed eth_dev can't be
opened back. Can you please add additional cleanup.

<...>

> diff --git a/drivers/net/ionic/ionic_osdep.h b/drivers/net/ionic/ionic_osdep.h
> index 5b8baa67b..39a9cefa9 100644
> --- a/drivers/net/ionic/ionic_osdep.h
> +++ b/drivers/net/ionic/ionic_osdep.h
> @@ -28,6 +28,8 @@
>  #define BIT_ULL(nr)        (1ULL << (nr))
>  #define BITS_TO_LONGS(nr)  div_round_up(nr, 8 * sizeof(long))
>  
> +#define ETH_ALEN        6

You can use 'RTE_ETHER_ADDR_LEN' instead in the net libary

  reply index

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-15  8:22 [dpdk-dev] [PATCH v2 00/17] Introduces net/ionic PMD Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 01/17] net/ionic: add skeleton Alfredo Cardigliano
2019-12-02 16:06   ` Ferruh Yigit
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 02/17] net/ionic: add hardware structures definitions Alfredo Cardigliano
2019-12-02 16:07   ` Ferruh Yigit
2019-12-02 16:33   ` Stephen Hemminger
2019-12-04 16:26     ` Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 03/17] net/ionic: add log Alfredo Cardigliano
2019-12-02 16:07   ` Ferruh Yigit
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 04/17] net/ionic: register and initialize the adapter Alfredo Cardigliano
2019-12-02 16:09   ` Ferruh Yigit
2019-12-08 19:25     ` Alfredo Cardigliano
2019-12-09  9:12       ` Ferruh Yigit
2019-12-02 16:35   ` Stephen Hemminger
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 05/17] net/ionic: add port management commands Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 06/17] net/ionic: add basic lif support Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 07/17] net/ionic: add doorbells Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 08/17] net/ionic: add adminq support Alfredo Cardigliano
2019-12-02 16:15   ` Ferruh Yigit
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 09/17] net/ionic: add notifyq support Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 10/17] net/ionic: add basic port operations Alfredo Cardigliano
2019-12-02 16:11   ` Ferruh Yigit [this message]
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 11/17] net/ionic: add RX filters support Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 12/17] net/ionic: add Flow Control support Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 13/17] net/ionic: add RX and TX handling Alfredo Cardigliano
2019-12-02 16:13   ` Ferruh Yigit
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 14/17] net/ionic: add RSS support Alfredo Cardigliano
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 15/17] net/ionic: add stats Alfredo Cardigliano
2019-12-02 16:14   ` Ferruh Yigit
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 16/17] net/ionic: add TX checksum support Alfredo Cardigliano
2019-12-02 16:15   ` Ferruh Yigit
2019-10-15  8:22 ` [dpdk-dev] [PATCH v2 17/17] net/ionic: read fw version Alfredo Cardigliano
2019-10-15 13:09 ` [dpdk-dev] [PATCH v2 00/17] Introduces net/ionic PMD Ferruh Yigit

Reply instructions:

You may reply publically 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=662c5427-7417-7ae5-b40a-5a7175f58e84@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=cardigliano@ntop.org \
    --cc=dev@dpdk.org \
    --cc=john.mcnamara@intel.com \
    --cc=marko.kovacevic@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

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git