On 7/7/21 6:23 AM, Zhang, Qi Z wrote:
>
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
>> Sent: Tuesday, July 6, 2021 4:05 PM
>> To: Zhang, Qi Z <qi.z.zh...@intel.com>; Zhang, AlvinX
>> <alvinx.zh...@intel.com>; ajit.khapa...@broadcom.com
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
>>
>> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Zhang, AlvinX <alvinx.zh...@intel.com>
>>>> Sent: Tuesday, July 6, 2021 3:06 PM
>>>> To: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>; Zhang, Qi Z
>>>> <qi.z.zh...@intel.com>; ajit.khapa...@broadcom.com
>>>> Cc: dev@dpdk.org
>>>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload
>>>> types
>>>>
>>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>>>>>> #define ETH_RSS_PPPOE (1ULL << 31)
>>>>>> #define ETH_RSS_ECPRI (1ULL << 32)
>>>>>> #define ETH_RSS_MPLS (1ULL << 33)
>>>>>> +#define ETH_RSS_IPV4_CHKSUM (1ULL << 34)
>>>>>> +#define ETH_RSS_L4_CHKSUM (1ULL << 35)
>>>>>
>>>>> What does efine which L4 protocols are supported? How user will know?
>>>>
>>>> I think if we want to support L4 checksum RSS by using below command
>>>> port config all rss (all|default|eth|vlan|...)
>>>>
>>>> We must define TCP/UDP/SCTP checksum RSS separately:
>>>> #define ETH_RSS_TCP_CHKSUM (1ULL << 35)
>>>> #define ETH_RSS_UDP_CHKSUM (1ULL << 36)
>>>> #deifne ETH_RSS_SCTP_CHKSUM (1ULL << 37)
>>>>
>>>> Here 3 bits are occupied, this is not good for there are not many bits
>> available.
>>>>
>>>> If we only want to using it in flows, we only need to define
>>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
>>>> protocol type.
>>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
>>>> types l4-chksum end queues end / end
>>>
>>> +1, the pattern already give the hint to avoid the ambiguity and I think we
>> already have ETH_RSS_LEVEL to figure out inner or outer.
>>
>> The problem that it may be used in generic RSS flags which has no the
>> context.
>> Also even in the case of flow API context could have no L4 protocol at all.
>
> For generic case, it can simply assume it cover all L4 checksum cases and I'm
> not sure if any user intend to use it as generic RSS, pmd can simply reject
> it if it's not necessary to support.
Try to look at it from an application point of view which does
not know any specifics of the driver.
* Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to
use it.
* If I try to use it in default RSS config, but the request
fail, it could be very confusing.
* Will it distribute TCP packets? UDP packets? SCTP packets?
Or should I care about RSS for some of them based on other
supported fields? E.g. if SCTP is not supported by the NIC,
I need to install RSS flow rule for the IP protocol to do
RSS based on IPv4/IPv6 addresses. But if SCTP is supported,
I'm happy to use ETH_RSS_L4_CHKSUM for it as well.
> In flow API, if no l4 protocol in pattern , the PMD should return failure (or
> maybe some default behavior), and I think this is not a new question as it
> happens all the cases
> e.g.:
> pattern eth / vlan / end action rss type ipv4 .
IMHO, it would be pretty logical to apply RSS to IPv4 packets
only and send everything else to default queue.
>>
>> Is UDP checksum 0 treated as no checksum and go to default queue or treated
>> as a regular checksum with value equal to 0?
>
> I think we can treat it as value 0, as least our hardware behavior like this,
> is this any issue?
OK, no problem. Just document it.
>>
>> I tend to agree that 3 flags is too much for the feature, but one flag
>> without
>> properly defined meaning is not good as well.
>>
>> I just want rules to be defined and documented.'
>
> Agree, we need more document for this. if you agree above proposal.
>
