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: <630e590e-fac3-5f69-688e-ac140ab3464e@huawei.com>
Date:   Mon, 17 Apr 2023 20:35:04 +0800
From:   Yunsheng Lin <linyunsheng@...wei.com>
To:     Mengyuan Lou <mengyuanlou@...-swift.com>, <netdev@...r.kernel.org>
CC:     <jiawenwu@...stnetic.com>
Subject: Re: [PATCH net-next v2 1/5] net: wangxun: libwx add tx offload
 functions

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?

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.