netdev - Re: [PATCH net-next v2 1/5] net: wangxun: libwx add tx offload functions

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: <4E862584-755D-4EC4-9588-DB0B14D64CD5@net-swift.com>
Date:   Tue, 18 Apr 2023 15:00:04 +0800
From:   "mengyuanlou@...-swift.com" <mengyuanlou@...-swift.com>
To:     Yunsheng Lin <linyunsheng@...wei.com>
Cc:     netdev@...r.kernel.org, Jiawen Wu <jiawenwu@...stnetic.com>
Subject: Re: [PATCH net-next v2 1/5] net: wangxun: libwx add tx offload
 functions



> 2023年4月17日 20:35，Yunsheng Lin <linyunsheng@...wei.com> 写道：
> 
> On 2023/4/17 18:54, Mengyuan Lou wrote:
>> +
>> +static u8 wx_encode_tx_desc_ptype(const struct wx_tx_buffer *first)
>> +{
>> + u8 tun_prot = 0, l4_prot = 0, ptype = 0;
>> + struct sk_buff *skb = first->skb;
>> +
>> + if (skb->encapsulation) {
>> + union network_header hdr;
>> +
>> + switch (first->protocol) {
>> + case htons(ETH_P_IP):
>> + tun_prot = ip_hdr(skb)->protocol;
>> + if (ip_is_fragment(ip_hdr(skb)))
>> + goto encap_frag;
> 
> It seems you can do below:
> if (ip_is_fragment(ip_hdr(skb)))
> return WX_PTYPE_PKT_IP | WX_PTYPE_TYP_IPFRAG;
> 
> Instead of goto encap_frag to do the first->protocol and
> ip_is_fragment() checking again?
> 
>> + ptype = WX_PTYPE_TUN_IPV4;
>> + break;
>> + case htons(ETH_P_IPV6):
>> + tun_prot = wx_get_ipv6_proto(skb, skb_network_offset(skb));
>> + if (tun_prot == NEXTHDR_FRAGMENT)
>> + goto encap_frag;
> 
> Similar handling here?
> 
>> + ptype = WX_PTYPE_TUN_IPV6;
>> + break;
>> + default:
>> + goto exit;
> 
> Return 0 or return ptype directly instead of goto?
> 
> Similar comment as other 'goto exit'.
> 
>> + }
>> +
>> + if (tun_prot == IPPROTO_IPIP) {
> 
> ...
> 
>> + goto exit;
>> + }
>> + } else {
>> +encap_frag:
>> + switch (first->protocol) {
>> + case htons(ETH_P_IP):
>> + l4_prot = ip_hdr(skb)->protocol;
>> + ptype = WX_PTYPE_PKT_IP;
>> + if (ip_is_fragment(ip_hdr(skb))) {
>> + ptype |= WX_PTYPE_TYP_IPFRAG;
>> + goto exit;
>> + }
>> + break;
>> + case htons(ETH_P_IPV6):
>> + l4_prot = wx_get_ipv6_proto(skb, skb_network_offset(skb));
>> + ptype = WX_PTYPE_PKT_IP | WX_PTYPE_PKT_IPV6;
>> + if (l4_prot == NEXTHDR_FRAGMENT) {
>> + ptype |= WX_PTYPE_TYP_IPFRAG;
>> + goto exit;
>> + }
>> + break;
>> + case htons(ETH_P_1588):
>> + ptype = WX_PTYPE_L2_TS;
>> + goto exit;
>> + case htons(ETH_P_FIP):
>> + ptype = WX_PTYPE_L2_FIP;
>> + goto exit;
>> + case htons(ETH_P_LLDP):
>> + ptype = WX_PTYPE_L2_LLDP;
>> + goto exit;
>> + case htons(ETH_P_CNM):
>> + ptype = WX_PTYPE_L2_CNM;
>> + goto exit;
>> + case htons(ETH_P_PAE):
>> + ptype = WX_PTYPE_L2_EAPOL;
>> + goto exit;
>> + case htons(ETH_P_ARP):
>> + ptype = WX_PTYPE_L2_ARP;
>> + goto exit;
>> + default:
>> + ptype = WX_PTYPE_L2_MAC;
> 
> Is it ok to set ptype to WX_PTYPE_L2_MAC for first->protocol != ETH_P_IP
> && first->protocol != ETH_P_IPV6? Does hw need to do checksum/tso or other thing
> about those packet? if not, setting WX_PTYPE_L2_MAC seems enough?
> 
    • The hardware needs to parse these packets with these ptype bits.
> Also, some macro and variable are still not used, such as WX_PTYPE_L2_FCOE_VFT_FCRDY
> and vft_shadow, it would be better to remove it for now, and add it as needed.
>