DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Bingbin Chen <chen.bingbin@zte.com.cn>
Cc: ivan.malov@arknetworks.am, wang.junlong1@zte.com.cn,
	yang.yonggang@zte.com.cn, dev@dpdk.org
Subject: Re: [PATCH v6 1/2] net/zxdh: npsdk add flow director table ops
Date: Wed, 20 Aug 2025 12:46:41 -0700	[thread overview]
Message-ID: <20250820124641.152b667c@hermes.local> (raw)
In-Reply-To: <20250815074221.2254545-2-chen.bingbin@zte.com.cn>

On Fri, 15 Aug 2025 15:42:16 +0800
Bingbin Chen <chen.bingbin@zte.com.cn> wrote:

> +static uint32_t
> +zxdh_np_agent_channel_acl_index_request(uint32_t dev_id, uint32_t sdt_no,
> +			uint32_t vport, uint32_t *p_index)
> +{
> +	uint32_t rc = ZXDH_OK;
> +
> +	uint32_t rsp_buff[2] = {0};
> +	uint32_t msg_result = 0;
> +	uint32_t acl_index = 0;
> +	ZXDH_AGENT_CHANNEL_ACL_MSG_T msgcfg = {
> +		.dev_id = 0,
> +		.type = ZXDH_ACL_MSG,
> +		.oper = ZXDH_ACL_INDEX_REQUEST,
> +		.vport = vport,
> +		.sdt_no = sdt_no,
> +	};
> +	ZXDH_AGENT_CHANNEL_MSG_T agent_msg = {
> +		.msg = (void *)&msgcfg,
> +		.msg_len = sizeof(ZXDH_AGENT_CHANNEL_ACL_MSG_T),
> +	};
> +
> +	rc = zxdh_np_agent_channel_sync_send(dev_id, &agent_msg, rsp_buff, sizeof(rsp_buff));

You were probably taught somewhere to initialize all variables. But that is actually
outdated recommendation. Modern compilers detect uninitialized variables and therefore bugs
rather than just getting what was set before.

For example here. The first declaration sets 'rc' but then the first call is then
setting rc, so the initialization in the declaration is redundent. I see lots of this in this code.

The best way in modern code is to put variable declaration as close
to where it is first used as possible.

	uint32_t rc = zxdh_np_agent_channel_sync_send(dev_id, &agent_msg, rsp_buff, sizeof(rsp_buff));

> +	dump_depth = dump_depth_128bit - start_index_128bit;
> +
> +	dump_data_buff = (uint8_t *)rte_zmalloc(NULL, dump_depth * ZXDH_DTB_LEN_POS_SETP, 0);

Since rte_zmalloc() returns void * cast here is not needed.
This extra cast is done lots of places.

Also, can this be from heap (malloc) or stack (alloca) instead? 
Or does it need to be in hugepages for DMA reasons?
Same for lots of other buffers in the dump code.



  reply	other threads:[~2025-08-20 19:46 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-17  9:31 [PATCH v1 0/2] add support flow director ops Bingbin Chen
2025-06-17  9:32 ` [PATCH v1 1/2] net/zxdh: npsdk add flow director table ops Bingbin Chen
2025-06-17 14:07   ` Stephen Hemminger
2025-06-17 14:08   ` Stephen Hemminger
2025-06-17  9:32 ` [PATCH v1 2/2] net/zxdh: add support flow director ops Bingbin Chen
2025-06-18  7:49 ` [PATCH v2 0/2] " Bingbin Chen
2025-06-18  7:49   ` [PATCH v2 1/2] net/zxdh: npsdk add flow director table ops Bingbin Chen
2025-06-18  7:49   ` [PATCH v2 2/2] net/zxdh: add support flow director ops Bingbin Chen
2025-06-30 16:56     ` Stephen Hemminger
2025-07-02  7:34   ` [PATCH v3 0/2] " Bingbin Chen
2025-07-02  7:34     ` [PATCH v3 1/2] net/zxdh: npsdk add flow director table ops Bingbin Chen
2025-07-02  7:34     ` [PATCH v3 2/2] net/zxdh: add support flow director ops Bingbin Chen
2025-07-02 15:02       ` Stephen Hemminger
2025-08-03 17:34       ` Stephen Hemminger
2025-08-08  7:10     ` [PATCH v4 0/2] " Junlong Wang
2025-08-08  7:10       ` [PATCH v4 1/2] net/zxdh: npsdk add flow director table ops Junlong Wang
2025-08-08  7:10       ` [PATCH v4 2/2] net/zxdh: add support flow director ops Junlong Wang
2025-08-08  9:15         ` Ivan Malov
2025-08-08 16:12         ` Stephen Hemminger
2025-08-12  1:23         ` [v4,2/2] " Junlong Wang
2025-08-12  4:04           ` Ivan Malov
2025-08-12  7:19         ` Junlong Wang
2025-08-12  7:36           ` Ivan Malov
2025-08-12 10:47         ` Junlong Wang
2025-08-14  2:52       ` [PATCH v5 0/2] " Bingbin Chen
2025-08-14  2:52         ` [PATCH v5 1/2] net/zxdh: npsdk add flow director table ops Bingbin Chen
2025-08-14  2:52         ` [PATCH v5 2/2] net/zxdh: add support flow director ops Bingbin Chen
2025-08-14 16:59           ` Stephen Hemminger
2025-08-15  1:33             ` fengchengwen
2025-08-15 16:01               ` Stephen Hemminger
2025-08-15  7:42         ` [PATCH v6 0/2] " Bingbin Chen
2025-08-15  7:42           ` [PATCH v6 1/2] net/zxdh: npsdk add flow director table ops Bingbin Chen
2025-08-20 19:46             ` Stephen Hemminger [this message]
2025-08-15  7:42           ` [PATCH v6 2/2] net/zxdh: add support flow director ops Bingbin Chen
2025-08-15  8:00             ` Ivan Malov
2025-08-15 15:23         ` [PATCH v5 0/2] " Patrick Robb

Reply instructions:

You may reply publicly 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=20250820124641.152b667c@hermes.local \
    --to=stephen@networkplumber.org \
    --cc=chen.bingbin@zte.com.cn \
    --cc=dev@dpdk.org \
    --cc=ivan.malov@arknetworks.am \
    --cc=wang.junlong1@zte.com.cn \
    --cc=yang.yonggang@zte.com.cn \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).