Re: [PATCH V2 net-next 0/6] net: hns3: enhance tc flow offload support

[Paolo Abeni <pabeni@redhat.com>]

On 5/28/26 1:24 PM, Jijie Shao wrote:
> on 2026/5/28 17:06, Paolo Abeni wrote:
>> This one:
>>
>> ---
>>> +		if (is_zero_ether_addr(match.key->dst))
>>> +			rule->unused_tuple |= BIT(INNER_DST_MAC);
>>> +		if (is_zero_ether_addr(match.key->src))
>>> +			rule->unused_tuple |= BIT(INNER_SRC_MAC);
>> Should the "is this tuple unused" decision be driven by the mask
>> rather than the key?
>> A tc-flower rule that explicitly matches an all-zero address, for
>> example:
>>      tc filter add ... flower dst_mac 00:00:00:00:00:00 ...
>>      tc filter add ... flower src_ip 0.0.0.0 ...
>>      tc filter add ... flower src_ip :: ...
>> produces match.key->{src,dst} == 0 with match.mask->{src,dst} set to
>> all-ones. With the new checks above, those rules now have
>> INNER_{SRC,DST}_MAC / INNER_{SRC,DST}_IP added to rule->unused_tuple
>> even though the user asked to match exactly that address.
>> Downstream, hclge_fd_convert_tuple() short-circuits on unused_tuple:
>>      if (rule->unused_tuple & BIT(tuple_bit))
>>          return true;
>> so the tuple is not programmed into the TCAM key and the field is
>> effectively wildcarded in hardware, with no -EINVAL or extack returned
>> to the user.
>> Real-world patterns that rely on all-zero addresses include DHCPv4
>> DISCOVER (src 0.0.0.0), IPv6 DAD/NUD probes and DHCPv6 (src ::), and
>> deliberate matches on the all-zero MAC.
>> ---
>>
>> and similar later ones look real/relevant to me, and IMHO should be
>> addressed.
> 
> Yes, I saw this issue. There is similar logic in the existing function hclge_fd_check_ether_tuple():
> 
> static int hclge_fd_check_ether_tuple(struct ethhdr *spec, u32 *unused_tuple)
> {
> 	if (!spec || !unused_tuple)
> 		return -EINVAL;
> 
> 	*unused_tuple |= BIT(INNER_SRC_IP) | BIT(INNER_DST_IP) |
> 		BIT(INNER_SRC_PORT) | BIT(INNER_DST_PORT) |
> 		BIT(INNER_IP_TOS) | BIT(INNER_IP_PROTO);
> 
> 	if (is_zero_ether_addr(spec->h_source))
> 		*unused_tuple |= BIT(INNER_SRC_MAC);
> 
> 	if (is_zero_ether_addr(spec->h_dest))
> 		*unused_tuple |= BIT(INNER_DST_MAC);
> 
> 	if (!spec->h_proto)
> 		*unused_tuple |= BIT(INNER_ETH_TYPE);
> 
> 	return 0;
> }
> 
> This looks like a common problem for hns3. My original idea was to send 
> a separate bugfix later to fix all the all-zero MAC or IP addresses.>
The current tc flow implementation logic is consistent with ethtool -U.
> 
> Alternatively, I can first fix the tc flow issue in v4, and then submit
> a bugfix later to fix the potential issues in ethtool -U.

I think the latter option (v4 using `mask` and follow-up patch for
existing code) is the better one (to avoid introducing new code with a
known issues).

Thanks,

Paolo