linux-kernel - Re: [PATCH net-next 02/12] net: hns3: enable DCB when TC num is one and pfc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+FuTSdWa0dMz15m79SLsNAw9zkp3+3MSfKiRwKnjZ7QAyq1Uw@mail.gmail.com>
Date:   Fri, 28 Jun 2019 14:47:57 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Huazhong Tan <tanhuazhong@...wei.com>
Cc:     David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        salil.mehta@...wei.com, yisen.zhuang@...wei.com,
        linuxarm@...wei.com, Yunsheng Lin <linyunsheng@...wei.com>,
        Peng Li <lipeng321@...wei.com>
Subject: Re: [PATCH net-next 02/12] net: hns3: enable DCB when TC num is one
 and pfc_en is non-zero

On Fri, Jun 28, 2019 at 7:53 AM Huazhong Tan <tanhuazhong@...wei.com> wrote:
>
> From: Yunsheng Lin <linyunsheng@...wei.com>
>
> Currently when TC num is one, the DCB will be disabled no matter if
> pfc_en is non-zero or not.
>
> This patch enables the DCB if pfc_en is non-zero, even when TC num
> is one.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> Signed-off-by: Peng Li <lipeng321@...wei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@...wei.com>

> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
> index 9edae5f..cb2fb5a 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
> @@ -597,8 +597,10 @@ static void hclge_tm_tc_info_init(struct hclge_dev *hdev)
>                 hdev->tm_info.prio_tc[i] =
>                         (i >= hdev->tm_info.num_tc) ? 0 : i;
>
> -       /* DCB is enabled if we have more than 1 TC */
> -       if (hdev->tm_info.num_tc > 1)
> +       /* DCB is enabled if we have more than 1 TC or pfc_en is
> +        * non-zero.
> +        */
> +       if (hdev->tm_info.num_tc > 1 || hdev->tm_info.pfc_en)

small nit: comments that just repeat the condition are not very informative.

More helpful might be to explain why the DCB should be enabled in both
these cases. Though such detailed comments, if useful, are better left
to the commit message usually.

>                 hdev->flag |= HCLGE_FLAG_DCB_ENABLE;
>         else
>                 hdev->flag &= ~HCLGE_FLAG_DCB_ENABLE;
> @@ -1388,6 +1390,19 @@ void hclge_tm_schd_info_update(struct hclge_dev *hdev, u8 num_tc)
>         hclge_tm_schd_info_init(hdev);
>  }
>
> +void hclge_tm_pfc_info_update(struct hclge_dev *hdev)
> +{
> +       /* DCB is enabled if we have more than 1 TC or pfc_en is
> +        * non-zero.
> +        */
> +       if (hdev->tm_info.num_tc > 1 || hdev->tm_info.pfc_en)
> +               hdev->flag |= HCLGE_FLAG_DCB_ENABLE;
> +       else
> +               hdev->flag &= ~HCLGE_FLAG_DCB_ENABLE;
> +
> +       hclge_pfc_info_init(hdev);
> +}

Avoid introducing this code duplication by defining a helper?