From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5D8C5426DE; Sat, 7 Oct 2023 10:18:07 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 32E6F4029C; Sat, 7 Oct 2023 10:18:07 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 8797F40293 for ; Sat, 7 Oct 2023 10:18:05 +0200 (CEST) Received: from dggpeml500011.china.huawei.com (unknown [172.30.72.57]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4S2dNw2DWBzNp3X; Sat, 7 Oct 2023 16:14:08 +0800 (CST) Received: from [10.67.121.193] (10.67.121.193) by dggpeml500011.china.huawei.com (7.185.36.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Sat, 7 Oct 2023 16:18:02 +0800 Message-ID: Date: Sat, 7 Oct 2023 16:18:02 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] app/testpmd: add flush multicast MAC address command Content-Language: en-US To: Ferruh Yigit , CC: , , , , References: <20230801024304.2187484-1-huangdengdui@huawei.com> <20230802062311.3477794-1-huangdengdui@huawei.com> From: huangdengdui In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.193] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpeml500011.china.huawei.com (7.185.36.84) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 : >> flush multicast MAC address on port_id >> >> Signed-off-by: Dengdui Huang >> --- >> 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 : " >> + "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.