From: huangdengdui <huangdengdui@huawei.com>
To: Ferruh Yigit <ferruh.yigit@amd.com>, <dev@dpdk.org>
Cc: <stephen@networkplumber.org>, <aman.deep.singh@intel.com>,
<yuying.zhang@intel.com>, <anatoly.burakov@intel.com>,
<liuyonglong@huawei.com>
Subject: Re: [PATCH v2] app/testpmd: add flush multicast MAC address command
Date: Sat, 7 Oct 2023 16:18:02 +0800 [thread overview]
Message-ID: <dbdb5e7e-9ec4-4a27-97c9-48bf11a93022@huawei.com> (raw)
In-Reply-To: <f75131b3-4863-4018-8f7b-f9aadeb30ca6@amd.com>
On 2023/9/29 1:23, Ferruh Yigit wrote:
> On 8/2/2023 7:23 AM, Dengdui Huang wrote:
>> Add command to flush multicast MAC address
>> Usage:
>> mcast_addr flush <port_id> :
>> flush multicast MAC address on port_id
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>> app/test-pmd/cmdline.c | 43 +++++++++++++++++++++
>> app/test-pmd/config.c | 18 +++++++++
>> app/test-pmd/testpmd.h | 1 +
>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8 ++++
>> 4 files changed, 70 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 0d0723f659..d5a3bd2287 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -516,6 +516,9 @@ static void cmd_help_long_parsed(void *parsed_result,
>> "set allmulti (port_id|all) (on|off)\n"
>> " Set the allmulti mode on port_id, or all.\n\n"
>>
>> + "mcast_addr flush (port_id)\n"
>> + " To flush the set of multicast addresses.\n\n"
>> +>
>
> I assume 'set of multicast address' comes from the API getting
> 'mc_addr_set' as parameter, but using 'set' is not useful in the command
> description, what about:
> "Flush all multicast MAC addresses on port_id."
>
> Similarly there are logs/documents below refers as "set of multicast
> addresses", does it make sense to replace all as "all multicast MAC
> addresses"?
>
>
> And relating to the ordering, as you said "mcast_addr add|remove ..." is
> missing, which is contributing to the confusion.
> Can you please add this first (in a separate patch) to just below
> "mac_addr .*" group, later put "mcast_addr flush .*' below it in this patch?
>
>
>> "set flow_ctrl rx (on|off) tx (on|off) (high_water)"
>> " (low_water) (pause_time) (send_xon) mac_ctrl_frame_fwd"
>> " (on|off) autoneg (on|off) (port_id)\n"
>> @@ -8561,6 +8564,45 @@ static cmdline_parse_inst_t cmd_mcast_addr = {
>> },
>> };
>>
>> +/* *** FLUSH MULTICAST MAC ADDRESS ON PORT *** */
>> +struct cmd_mcast_addr_flush_result {
>> + cmdline_fixed_string_t mcast_addr_cmd;
>> + cmdline_fixed_string_t what;
>> + uint16_t port_num;
>> +};
>> +
>> +static void cmd_mcast_addr_flush_parsed(void *parsed_result,
>> + __rte_unused struct cmdline *cl,
>> + __rte_unused void *data)
>> +{
>> + struct cmd_mcast_addr_flush_result *res = parsed_result;
>> +
>> + mcast_addr_flush(res->port_num);
>> +}
>> +
>> +static cmdline_parse_token_string_t cmd_mcast_addr_flush_cmd =
>> + TOKEN_STRING_INITIALIZER(struct cmd_mcast_addr_result,
>> + mcast_addr_cmd, "mcast_addr");
>> +static cmdline_parse_token_string_t cmd_mcast_addr_flush_what =
>> + TOKEN_STRING_INITIALIZER(struct cmd_mcast_addr_result, what,
>> + "flush");
>> +static cmdline_parse_token_num_t cmd_mcast_addr_flush_portnum =
>> + TOKEN_NUM_INITIALIZER(struct cmd_mcast_addr_result, port_num,
>> + RTE_UINT16);
>> +
>> +static cmdline_parse_inst_t cmd_mcast_addr_flush = {
>> + .f = cmd_mcast_addr_flush_parsed,
>> + .data = (void *)0,
>> + .help_str = "mcast_addr flush <port_id> : "
>> + "flush multicast MAC address on port_id",>
>
> What about:
> "Flush all multicast MAC addresses on port_id."
>
>
>> + .tokens = {
>> + (void *)&cmd_mcast_addr_flush_cmd,
>> + (void *)&cmd_mcast_addr_flush_what,
>> + (void *)&cmd_mcast_addr_flush_portnum,
>> + NULL,
>> + },
>> +};
>> +
>> /* vf vlan anti spoof configuration */
>>
>> /* Common result structure for vf vlan anti spoof */
>> @@ -12929,6 +12971,7 @@ static cmdline_parse_ctx_t builtin_ctx[] = {
>> (cmdline_parse_inst_t *)&cmd_set_port_meter_stats_mask,
>> (cmdline_parse_inst_t *)&cmd_show_port_meter_stats,
>> (cmdline_parse_inst_t *)&cmd_mcast_addr,
>> + (cmdline_parse_inst_t *)&cmd_mcast_addr_flush,
>> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_anti_spoof,
>> (cmdline_parse_inst_t *)&cmd_set_vf_mac_anti_spoof,
>> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_stripq,
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 11f3a22048..d66d1db37c 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -6802,6 +6802,24 @@ mcast_addr_remove(portid_t port_id, struct rte_ether_addr *mc_addr)
>> mcast_addr_pool_append(port, mc_addr);
>> }
>>
>> +void
>> +mcast_addr_flush(portid_t port_id)
>> +{
>> + int ret;
>> +
>> + if (port_id_is_invalid(port_id, ENABLED_WARN))
>> + return;
>> +
>> + ret = rte_eth_dev_set_mc_addr_list(port_id, NULL, 0);
>> + if (ret != 0) {
>> + fprintf(stderr,
>> + "Failed to flush the set of filtered addresses on port %u\n",
>> + port_id);
>> + return;
>> + }
>> + mcast_addr_pool_destroy(port_id);
>> +}
>> +
>> void
>> port_dcb_info_display(portid_t port_id)
>> {
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index f1df6a8faf..65d3634f6a 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -1176,6 +1176,7 @@ void show_mcast_macs(portid_t port_id);
>> /* Functions to manage the set of filtered Multicast MAC addresses */
>> void mcast_addr_add(portid_t port_id, struct rte_ether_addr *mc_addr);
>> void mcast_addr_remove(portid_t port_id, struct rte_ether_addr *mc_addr);
>> +void mcast_addr_flush(portid_t port_id);
>> void port_dcb_info_display(portid_t port_id);
>>
>> uint8_t *open_file(const char *file_path, uint32_t *size);
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index a182479ab2..c3ab1507c4 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -1320,6 +1320,14 @@ filtered by port::
>>
>> testpmd> mcast_addr remove (port_id) (mcast_addr)
>>
>> +mcast_addr flush
>> +~~~~~~~~~~~~~~~~
>> +
>> +To flush the set of multicast addresses
>> +filtered by port::
>>
>
> Can join the lines into single line
>
>> +
>> + testpmd> mcast_addr flush (port_id)
>> +
>> mac_addr add (for VF)
>> ~~~~~~~~~~~~~~~~~~~~~
>>
>
Thanks for your review, I will do in v3.
next prev parent reply other threads:[~2023-10-07 8:18 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-01 2:43 [PATCH] " Dengdui Huang
2023-08-01 15:18 ` Stephen Hemminger
2023-08-02 1:28 ` huangdengdui
2023-08-02 2:41 ` huangdengdui
2023-08-02 4:16 ` Stephen Hemminger
2023-08-02 6:11 ` huangdengdui
2023-08-02 6:23 ` [PATCH v2] " Dengdui Huang
2023-09-28 17:23 ` Ferruh Yigit
2023-10-07 8:18 ` huangdengdui [this message]
2023-10-08 1:56 ` [PATCH v3 0/2] fix help string and add a new command Dengdui Huang
2023-10-08 1:56 ` [PATCH v3 1/2] app/testpmd: fix help string Dengdui Huang
2023-10-08 2:47 ` fengchengwen
2023-10-08 6:46 ` huangdengdui
2023-10-08 1:56 ` [PATCH v3 2/2] app/testpmd: add flush all multicast MAC address command Dengdui Huang
2023-10-08 2:50 ` fengchengwen
2023-10-08 6:46 ` [PATCH v3 0/2] fix help string and add a new command Dengdui Huang
2023-10-08 6:46 ` [PATCH v4 1/2] app/testpmd: fix help string Dengdui Huang
2023-10-08 6:46 ` [PATCH v4 2/2] app/testpmd: add flush all multicast MAC address command Dengdui Huang
2023-10-09 9:39 ` [PATCH v3 0/2] fix help string and add a new command Ferruh Yigit
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=dbdb5e7e-9ec4-4a27-97c9-48bf11a93022@huawei.com \
--to=huangdengdui@huawei.com \
--cc=aman.deep.singh@intel.com \
--cc=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=liuyonglong@huawei.com \
--cc=stephen@networkplumber.org \
--cc=yuying.zhang@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
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).