[PATCH v7 1/8] net/gve/base: introduce base code

[Guo, Junfeng]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit at amd.com>
> Sent: Monday, October 24, 2022 18:51
> To: Guo, Junfeng <junfeng.guo at intel.com>; Zhang, Qi Z
> <qi.z.zhang at intel.com>; Wu, Jingjing <jingjing.wu at intel.com>;
> ferruh.yigit at xilinx.com; Xing, Beilei <beilei.xing at intel.com>
> Cc: dev at dpdk.org; Li, Xiaoyun <xiaoyun.li at intel.com>;
> awogbemila at google.com; Richardson, Bruce
> <bruce.richardson at intel.com>; hemant.agrawal at nxp.com;
> stephen at networkplumber.org; Xia, Chenbo <chenbo.xia at intel.com>;
> Zhang, Helin <helin.zhang at intel.com>; Wang, Haiyue
> <haiyue.wang at intel.com>
> Subject: Re: [PATCH v7 1/8] net/gve/base: introduce base code
> 
> On 10/21/2022 10:19 AM, Junfeng Guo wrote:
> > +int gve_adminq_describe_device(struct gve_priv *priv)
> > +{
> > +	struct gve_device_option_jumbo_frames
> *dev_op_jumbo_frames = NULL;
> > +	struct gve_device_option_gqi_rda *dev_op_gqi_rda = NULL;
> > +	struct gve_device_option_gqi_qpl *dev_op_gqi_qpl = NULL;
> > +	struct gve_device_option_dqo_rda *dev_op_dqo_rda = NULL;
> > +	struct gve_device_descriptor *descriptor;
> > +	struct gve_dma_mem descriptor_dma_mem;
> > +	u32 supported_features_mask = 0;
> > +	union gve_adminq_command cmd;
> > +	int err = 0;
> > +	u8 *mac;
> > +	u16 mtu;
> > +
> > +	memset(&cmd, 0, sizeof(cmd));
> > +	descriptor = gve_alloc_dma_mem(&descriptor_dma_mem,
> PAGE_SIZE);
> > +	if (!descriptor)
> > +		return -ENOMEM;
> > +	cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESCRIBE_DEVICE);
> > +	cmd.describe_device.device_descriptor_addr =
> > +
> 	cpu_to_be64(descriptor_dma_mem.pa);
> > +	cmd.describe_device.device_descriptor_version =
> > +
> 	cpu_to_be32(GVE_ADMINQ_DEVICE_DESCRIPTOR_VERSION);
> > +	cmd.describe_device.available_length = cpu_to_be32(PAGE_SIZE);
> > +
> > +	err = gve_adminq_execute_cmd(priv, &cmd);
> > +	if (err)
> > +		goto free_device_descriptor;
> > +
> > +	err = gve_process_device_options(priv, descriptor,
> &dev_op_gqi_rda,
> > +					 &dev_op_gqi_qpl,
> &dev_op_dqo_rda,
> > +					 &dev_op_jumbo_frames);
> > +	if (err)
> > +		goto free_device_descriptor;
> > +
> > +	/* If the GQI_RAW_ADDRESSING option is not enabled and the
> queue format
> > +	 * is not set to GqiRda, choose the queue format in a priority
> order:
> > +	 * DqoRda, GqiRda, GqiQpl. Use GqiQpl as default.
> > +	 */
> > +	if (dev_op_dqo_rda) {
> > +		priv->queue_format = GVE_DQO_RDA_FORMAT;
> > +		PMD_DRV_LOG(INFO, "Driver is running with DQO RDA
> queue format.");
> > +		supported_features_mask =
> > +			be32_to_cpu(dev_op_dqo_rda-
> >supported_features_mask);
> > +	} else if (dev_op_gqi_rda) {
> > +		priv->queue_format = GVE_GQI_RDA_FORMAT;
> > +		PMD_DRV_LOG(INFO, "Driver is running with GQI RDA
> queue format.");
> > +		supported_features_mask =
> > +			be32_to_cpu(dev_op_gqi_rda-
> >supported_features_mask);
> > +	} else if (priv->queue_format == GVE_GQI_RDA_FORMAT) {
> > +		PMD_DRV_LOG(INFO, "Driver is running with GQI RDA
> queue format.");
> > +	} else {
> > +		priv->queue_format = GVE_GQI_QPL_FORMAT;
> > +		if (dev_op_gqi_qpl)
> > +			supported_features_mask =
> > +				be32_to_cpu(dev_op_gqi_qpl-
> >supported_features_mask);
> > +		PMD_DRV_LOG(INFO, "Driver is running with GQI QPL
> queue format.");
> > +	}
> > +	if (gve_is_gqi(priv)) {
> > +		err = gve_set_desc_cnt(priv, descriptor);
> > +	} else {
> > +		/* DQO supports LRO. */
> > +		err = gve_set_desc_cnt_dqo(priv, descriptor,
> dev_op_dqo_rda);
> > +	}
> > +	if (err)
> > +		goto free_device_descriptor;
> > +
> > +	priv->max_registered_pages =
> > +				be64_to_cpu(descriptor-
> >max_registered_pages);
> > +	mtu = be16_to_cpu(descriptor->mtu);
> > +	if (mtu < ETH_MIN_MTU) {
> > +		PMD_DRV_LOG(ERR, "MTU %d below minimum MTU",
> mtu);
> > +		err = -EINVAL;
> > +		goto free_device_descriptor;
> > +	}
> > +	priv->max_mtu = mtu;
> > +	priv->num_event_counters = be16_to_cpu(descriptor->counters);
> > +	rte_memcpy(priv->dev_addr.addr_bytes, descriptor->mac,
> ETH_ALEN);
> > +	mac = descriptor->mac;
> > +	PMD_DRV_LOG(INFO, "MAC
> addr: %02x:%02x:%02x:%02x:%02x:%02x",
> > +		    mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
> 
> 
> There are 'RTE_ETHER_ADDR_PRT_FMT' & 'RTE_ETHER_ADDR_BYTES'
> macros for
> this purpose, you can use it like:
> 
> PMD_DRV_LOG(INFO, "MAC addr" RTE_ETHER_ADDR_PRT_FMT,
> RTE_ETHER_ADDR_BYTES(priv->dev_addr));
> 
> So can get rid of 'mac' variable.

Thanks for the advice! Will try this in the coming version.