From: Jakub Kicinski <kuba@kernel•org>
To: shaojijie@huawei•com
Cc: Jakub Kicinski <kuba@kernel•org>,
davem@davemloft•net, edumazet@google•com, pabeni@redhat•com,
andrew+netdev@lunn•ch, horms@kernel•org, shenjian15@huawei•com,
liuyonglong@huawei•com, chenhao418@huawei•com,
huangdonghua3@h-partners•com, yangshuaisong@h-partners•com,
netdev@vger•kernel.org, linux-kernel@vger•kernel.org
Subject: Re: [PATCH V3 net-next 4/6] net: hns3: support IP and tunnel VNI dissectors for tc flow
Date: Fri, 5 Jun 2026 17:44:55 -0700 [thread overview]
Message-ID: <20260606004455.4016108-1-kuba@kernel.org> (raw)
In-Reply-To: <20260602133337.2216918-5-shaojijie@huawei.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: hns3: support IP and tunnel VNI dissectors for tc flow
This patch adds support for FLOW_DISSECTOR_KEY_IP for ip_tos and
FLOW_DISSECTOR_KEY_ENC_KEYID for outer_tun_vni in the HNS3 driver's TC flow
dissection. It also explicitly marks user defined tuples as unused.
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index aa85abda29096..3559b471b3feb 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
[ ... ]
> @@ -5760,6 +5763,17 @@ static bool hclge_fd_convert_tuple(u32 tuple_bit, u8 *key_x, u8 *key_y,
> *(__le32 *)key_x = cpu_to_le32(tmp_x_l);
> *(__le32 *)key_y = cpu_to_le32(tmp_y_l);
>
> + return true;
> + case KEY_OPT_VNI:
> + calc_x(tmp_x_l, *(u32 *)(&p[offset]), *(u32 *)(&p[moffset]));
> + calc_y(tmp_y_l, *(u32 *)(&p[offset]), *(u32 *)(&p[moffset]));
> + for (i = 0; i < HCLGE_VNI_LENGTH; i++) {
> + key_x[i] = ((__force u32)cpu_to_le32(tmp_x_l) >>
> + (i * BITS_PER_BYTE)) & 0xFF;
> + key_y[i] = ((__force u32)cpu_to_le32(tmp_y_l) >>
> + (i * BITS_PER_BYTE)) & 0xFF;
[Severity: Medium]
On big-endian architectures, cpu_to_le32() byte-swaps the integer. If we
apply an arithmetic right-shift to a byte-swapped integer here, won't it
extract the bytes in the wrong order and drop the least significant byte?
For example, a 24-bit VNI of 0x123456 becomes 0x56341200 after swapping.
Shifting it right extracts 0x00, 0x12, and 0x34, entirely missing the 0x56
byte. Should this code shift the host-endian integer directly before
applying the mask?
> + }
> +
> return true;
> default:
> return false;
[ ... ]
> @@ -7340,6 +7354,53 @@ static void hclge_get_cls_key_port(const struct flow_rule *flow,
[ ... ]
> +static void hclge_get_cls_key_ip_tos(const struct flow_rule *flow,
> + struct hclge_fd_rule *rule)
> +{
> + if (flow_rule_match_key(flow, FLOW_DISSECTOR_KEY_IP)) {
> + struct flow_match_ip match;
> +
> + flow_rule_match_ip(flow, &match);
> +
> + rule->tuples.ip_tos = match.key->tos;
> + rule->tuples_mask.ip_tos = match.mask->tos;
> + if (!rule->tuples_mask.ip_tos)
> + rule->unused_tuple |= BIT(INNER_IP_TOS);
[Severity: High]
Since FLOW_DISSECTOR_KEY_IP is now added to the allowed keys list in
hclge_check_cls_flower(), the networking stack could pass rules containing
TTL constraints (match.mask->ttl).
Does this code silently ignore match.mask->ttl?
If the user requests a specific TTL match, the driver appears to successfully
extract the tos field but completely ignores the ttl constraint. Could this
cause the hardware filter to match broader traffic than intended, essentially
bypassing the user's intent? Should it return an error if an unsupported
TTL match is requested?
> + } else {
> + rule->unused_tuple |= BIT(INNER_IP_TOS);
> + }
> +}
next prev parent reply other threads:[~2026-06-06 0:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-02 13:33 [PATCH V3 net-next 0/6] net: hns3: enhance tc flow offload support Jijie Shao
2026-06-02 13:33 ` [PATCH V3 net-next 1/6] net: hns3: adjust add_cls_flower() for support more action Jijie Shao
2026-06-02 13:33 ` [PATCH V3 net-next 2/6] net: hns3: improve the unused_tuple parameter setting Jijie Shao
2026-06-02 13:33 ` [PATCH V3 net-next 3/6] net: hns3: support two more actions for tc flow Jijie Shao
2026-06-06 0:40 ` Jakub Kicinski
2026-06-02 13:33 ` [PATCH V3 net-next 4/6] net: hns3: support IP and tunnel VNI dissectors " Jijie Shao
2026-06-06 0:44 ` Jakub Kicinski [this message]
2026-06-02 13:33 ` [PATCH V3 net-next 5/6] net: hns3: debugfs support for dumping fd rules Jijie Shao
2026-06-02 13:33 ` [PATCH V3 net-next 6/6] net: hns3: move fd code to a separate file Jijie Shao
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=20260606004455.4016108-1-kuba@kernel.org \
--to=kuba@kernel$(echo .)org \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=chenhao418@huawei$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=huangdonghua3@h-partners$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=liuyonglong@huawei$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=shaojijie@huawei$(echo .)com \
--cc=shenjian15@huawei$(echo .)com \
--cc=yangshuaisong@h-partners$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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