From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
To: Huisong Li <lihuisong@huawei.com>
Cc: stable@dpdk.org, humin29@huawei.com
Subject: Re: [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC
Date: Mon, 10 Jan 2022 08:45:36 +0100 [thread overview]
Message-ID: <CAATJJ0KiR3UQBGqtLbj7V-Y0LHaezjyMYRtbtjs=mEv6078TmQ@mail.gmail.com> (raw)
In-Reply-To: <20211225105344.28355-2-lihuisong@huawei.com>
On Sat, Dec 25, 2021 at 11:58 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> [ upstream commit 19e67d8ebced5cb12829f75c70e6c497b5925e82 ]
>
Thank you, queued for 19.11.12 (later this year)
> This problem occurs in the following scenarios:
> 1) reset is encountered when the adapter is running.
> 2) set a new default MAC address.
>
> After the above two steps, the old default MAC address should be not
> take effect. But the current behavior is contrary to that. This is due
> to the change of the "default_addr_setted" in hw->mac from 'true' to
> 'false' after the reset. As a result, the old MAC address is not removed
> when the new default MAC address is set. This variable controls whether
> to delete the old default MAC address when setting the default MAC
> address. It is only used when the mac_addr_set API is called for the
> first time. In fact, when a unicast MAC address is deleted, if the
> address isn't in the MAC address table, the driver doesn't return
> failure. So this patch remove the redundant and troublesome variables to
> resolve this problem.
>
> Fixes: 7d7f9f80bbfb ("net/hns3: support MAC address related operations")
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> drivers/net/hns3/hns3_ethdev.c | 58 ++++++++++------------------------
> drivers/net/hns3/hns3_ethdev.h | 1 -
> 2 files changed, 16 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 157dd34d4f..a2676f84b0 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -1545,7 +1545,7 @@ hns3_remove_mc_addr_common(struct hns3_hw *hw, struct rte_ether_addr *mac_addr)
>
> static int
> hns3_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr,
> - uint32_t idx, __attribute__ ((unused)) uint32_t pool)
> + __rte_unused uint32_t idx, __rte_unused uint32_t pool)
> {
> struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> char mac_str[RTE_ETHER_ADDR_FMT_SIZE];
> @@ -1576,8 +1576,6 @@ hns3_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr,
> return ret;
> }
>
> - if (idx == 0)
> - hw->mac.default_addr_setted = true;
> rte_spinlock_unlock(&hw->lock);
>
> return ret;
> @@ -1642,35 +1640,18 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
> struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> struct rte_ether_addr *oaddr;
> char mac_str[RTE_ETHER_ADDR_FMT_SIZE];
> - bool default_addr_setted;
> - bool rm_succes = false;
> int ret, ret_val;
>
> - /* check if mac addr is valid */
> - if (!rte_is_valid_assigned_ether_addr(mac_addr)) {
> - rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> - mac_addr);
> - hns3_err(hw, "Failed to set mac addr, addr(%s) invalid",
> - mac_str);
> - return -EINVAL;
> - }
> -
> - oaddr = (struct rte_ether_addr *)hw->mac.mac_addr;
> - default_addr_setted = hw->mac.default_addr_setted;
> - if (default_addr_setted && !!rte_is_same_ether_addr(mac_addr, oaddr))
> - return 0;
> -
> rte_spinlock_lock(&hw->lock);
> - if (default_addr_setted) {
> - ret = hns3_remove_uc_addr_common(hw, oaddr);
> - if (ret) {
> - rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> - oaddr);
> - hns3_warn(hw, "Remove old uc mac address(%s) fail: %d",
> - mac_str, ret);
> - rm_succes = false;
> - } else
> - rm_succes = true;
> + oaddr = (struct rte_ether_addr *)hw->mac.mac_addr;
> + ret = hns3_remove_uc_addr_common(hw, oaddr);
> + if (ret) {
> + rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> + oaddr);
> + hns3_warn(hw, "Remove old uc mac address(%s) fail: %d",
> + mac_str, ret);
> + rte_spinlock_unlock(&hw->lock);
> + return ret;
> }
>
> ret = hns3_add_uc_addr_common(hw, mac_addr);
> @@ -1689,7 +1670,6 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
>
> rte_ether_addr_copy(mac_addr,
> (struct rte_ether_addr *)hw->mac.mac_addr);
> - hw->mac.default_addr_setted = true;
> rte_spinlock_unlock(&hw->lock);
>
> return 0;
> @@ -1705,16 +1685,12 @@ hns3_set_default_mac_addr(struct rte_eth_dev *dev,
> }
>
> err_add_uc_addr:
> - if (rm_succes) {
> - ret_val = hns3_add_uc_addr_common(hw, oaddr);
> - if (ret_val) {
> - rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> - oaddr);
> - hns3_warn(hw,
> - "Failed to restore old uc mac addr(%s): %d",
> - mac_str, ret_val);
> - hw->mac.default_addr_setted = false;
> - }
> + ret_val = hns3_add_uc_addr_common(hw, oaddr);
> + if (ret_val) {
> + rte_ether_format_addr(mac_str, RTE_ETHER_ADDR_FMT_SIZE,
> + oaddr);
> + hns3_warn(hw, "Failed to restore old uc mac addr(%s): %d",
> + mac_str, ret_val);
> }
> rte_spinlock_unlock(&hw->lock);
>
> @@ -2880,7 +2856,6 @@ hns3_get_board_configuration(struct hns3_hw *hw)
> hw->rss_dis_flag = false;
> memcpy(hw->mac.mac_addr, cfg.mac_addr, RTE_ETHER_ADDR_LEN);
> hw->mac.phy_addr = cfg.phy_addr;
> - hw->mac.default_addr_setted = false;
> hw->num_tx_desc = cfg.tqp_desc_num;
> hw->num_rx_desc = cfg.tqp_desc_num;
> hw->dcb_info.num_pg = 1;
> @@ -4699,7 +4674,6 @@ hns3_do_stop(struct hns3_adapter *hns)
> reset_queue = true;
> } else
> reset_queue = false;
> - hw->mac.default_addr_setted = false;
> return hns3_stop_queues(hns, reset_queue);
> }
>
> diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
> index 6c3ac6f8a9..be0fac4fd2 100644
> --- a/drivers/net/hns3/hns3_ethdev.h
> +++ b/drivers/net/hns3/hns3_ethdev.h
> @@ -144,7 +144,6 @@ enum hns3_media_type {
>
> struct hns3_mac {
> uint8_t mac_addr[RTE_ETHER_ADDR_LEN];
> - bool default_addr_setted; /* whether default addr(mac_addr) is set */
> uint8_t media_type;
> uint8_t phy_addr;
> uint8_t link_duplex : 1; /* ETH_LINK_[HALF/FULL]_DUPLEX */
> --
> 2.33.0
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
next prev parent reply other threads:[~2022-01-10 7:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-25 10:53 [PATCH 19.11 0/4] fix some bugs for hns3 Huisong Li
2021-12-25 10:53 ` [PATCH 19.11 1/4] net/hns3: fix residual MAC after setting default MAC Huisong Li
2022-01-10 7:45 ` Christian Ehrhardt [this message]
2021-12-25 10:53 ` [PATCH 19.11 2/4] net/hns3: fix secondary process reference count Huisong Li
2022-01-10 7:45 ` Christian Ehrhardt
2021-12-25 10:53 ` [PATCH 19.11 3/4] net/hns3: fix multi-process action register and unregister Huisong Li
2022-01-10 7:46 ` Christian Ehrhardt
2021-12-25 10:53 ` [PATCH 19.11 4/4] net/hns3: unregister MP action on close for secondary Huisong Li
2022-01-10 7:45 ` Christian Ehrhardt
2022-01-03 11:36 ` [PATCH 19.11 0/4] fix some bugs for hns3 Christian Ehrhardt
2022-01-04 2:50 ` lihuisong (C)
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='CAATJJ0KiR3UQBGqtLbj7V-Y0LHaezjyMYRtbtjs=mEv6078TmQ@mail.gmail.com' \
--to=christian.ehrhardt@canonical.com \
--cc=humin29@huawei.com \
--cc=lihuisong@huawei.com \
--cc=stable@dpdk.org \
/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).