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]
Date: Tue, 6 Jun 2023 11:52:55 +0800
From: Hao Lan <lanhao@...wei.com>
To: <edward.cree@....com>, <linux-net-drivers@....com>, <davem@...emloft.net>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <edumazet@...gle.com>
CC: Edward Cree <ecree.xilinx@...il.com>, <netdev@...r.kernel.org>,
	<habetsm.xilinx@...il.com>
Subject: Re: [PATCH net-next 6/6] sfc: generate encap headers for TC offload



On 2023/6/6 3:17, edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> Support constructing VxLAN and GENEVE headers, on either IPv4 or IPv6,
>  using the neighbouring information obtained in encap->neigh to
>  populate the Ethernet header.
> Note that the ef100 hardware does not insert UDP checksums when
>  performing encap, so for IPv6 the remote endpoint will need to be
>  configured with udp6zerocsumrx or equivalent.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
> ---
>  drivers/net/ethernet/sfc/tc_encap_actions.c | 194 +++++++++++++++++++-
>  1 file changed, 185 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/tc_encap_actions.c b/drivers/net/ethernet/sfc/tc_encap_actions.c
> index 601141190f42..9a51d91d16bd 100644
> --- a/drivers/net/ethernet/sfc/tc_encap_actions.c
> +++ b/drivers/net/ethernet/sfc/tc_encap_actions.c
> @@ -239,12 +239,183 @@ static void efx_release_neigh(struct efx_nic *efx,
>  	efx_free_neigh(neigh);
>  }
>  
> -static void efx_gen_encap_header(struct efx_tc_encap_action *encap)
> +static void efx_gen_tun_header_eth(struct efx_tc_encap_action *encap, u16 proto)
>  {
> -	/* stub for now */
> -	encap->n_valid = false;
> -	memset(encap->encap_hdr, 0, sizeof(encap->encap_hdr));
> -	encap->encap_hdr_len = ETH_HLEN;
> +	struct efx_neigh_binder *neigh = encap->neigh;
> +	struct ethhdr *eth;
> +
> +	encap->encap_hdr_len = sizeof(*eth);
> +	eth = (struct ethhdr *)encap->encap_hdr;
> +
> +	if (encap->neigh->n_valid)
> +		ether_addr_copy(eth->h_dest, neigh->ha);
> +	else
> +		eth_zero_addr(eth->h_dest);
> +	ether_addr_copy(eth->h_source, neigh->egdev->dev_addr);
> +	eth->h_proto = htons(proto);
> +}
> +
> +static void efx_gen_tun_header_ipv4(struct efx_tc_encap_action *encap, u8 ipproto, u8 len)
> +{
> +	struct efx_neigh_binder *neigh = encap->neigh;
> +	struct ip_tunnel_key *key = &encap->key;
> +	struct iphdr *ip;
> +
> +	ip = (struct iphdr *)(encap->encap_hdr + encap->encap_hdr_len);
> +	encap->encap_hdr_len += sizeof(*ip);
> +
> +	ip->daddr = key->u.ipv4.dst;
> +	ip->saddr = key->u.ipv4.src;
> +	ip->ttl = neigh->ttl;
> +	ip->protocol = ipproto;
> +	ip->version = 0x4;
> +	ip->ihl = 0x5;
> +	ip->tot_len = cpu_to_be16(ip->ihl * 4 + len);
> +	ip_send_check(ip);
> +}
> +
> +#ifdef CONFIG_IPV6
> +static void efx_gen_tun_header_ipv6(struct efx_tc_encap_action *encap, u8 ipproto, u8 len)
> +{
> +	struct efx_neigh_binder *neigh = encap->neigh;
> +	struct ip_tunnel_key *key = &encap->key;
> +	struct ipv6hdr *ip;
> +
> +	ip = (struct ipv6hdr *)(encap->encap_hdr + encap->encap_hdr_len);
> +	encap->encap_hdr_len += sizeof(*ip);
> +
> +	ip6_flow_hdr(ip, key->tos, key->label);
> +	ip->daddr = key->u.ipv6.dst;
> +	ip->saddr = key->u.ipv6.src;
> +	ip->hop_limit = neigh->ttl;
> +	ip->nexthdr = ipproto;
> +	ip->version = 0x6;
> +	ip->payload_len = cpu_to_be16(len);
> +}
> +#endif
> +
> +static void efx_gen_tun_header_udp(struct efx_tc_encap_action *encap, u8 len)
> +{
> +	struct ip_tunnel_key *key = &encap->key;
> +	struct udphdr *udp;
> +
> +	udp = (struct udphdr *)(encap->encap_hdr + encap->encap_hdr_len);
> +	encap->encap_hdr_len += sizeof(*udp);
> +
> +	udp->dest = key->tp_dst;
> +	udp->len = cpu_to_be16(sizeof(*udp) + len);
> +}
> +
> +static void efx_gen_tun_header_vxlan(struct efx_tc_encap_action *encap)
> +{
> +	struct ip_tunnel_key *key = &encap->key;
> +	struct vxlanhdr *vxlan;
> +
> +	vxlan = (struct vxlanhdr *)(encap->encap_hdr + encap->encap_hdr_len);
> +	encap->encap_hdr_len += sizeof(*vxlan);
> +
> +	vxlan->vx_flags = VXLAN_HF_VNI;
> +	vxlan->vx_vni = vxlan_vni_field(tunnel_id_to_key32(key->tun_id));
> +}
> +
> +static void efx_gen_tun_header_geneve(struct efx_tc_encap_action *encap)
> +{
> +	struct ip_tunnel_key *key = &encap->key;
> +	struct genevehdr *geneve;
> +	u32 vni;
> +
> +	geneve = (struct genevehdr *)(encap->encap_hdr + encap->encap_hdr_len);
> +	encap->encap_hdr_len += sizeof(*geneve);
> +
> +	geneve->proto_type = htons(ETH_P_TEB);
> +	/* convert tun_id to host-endian so we can use host arithmetic to
> +	 * extract individual bytes.
> +	 */
> +	vni = ntohl(tunnel_id_to_key32(key->tun_id));
> +	geneve->vni[0] = vni >> 16;
> +	geneve->vni[1] = vni >> 8;
> +	geneve->vni[2] = vni;
> +}
> +
> +#define vxlan_header_l4_len	(sizeof(struct udphdr) + sizeof(struct vxlanhdr))
> +#define vxlan4_header_len	(sizeof(struct ethhdr) + sizeof(struct iphdr) + vxlan_header_l4_len)
> +static void efx_gen_vxlan_header_ipv4(struct efx_tc_encap_action *encap)
> +{
> +	BUILD_BUG_ON(sizeof(encap->encap_hdr) < vxlan4_header_len);
> +	efx_gen_tun_header_eth(encap, ETH_P_IP);
> +	efx_gen_tun_header_ipv4(encap, IPPROTO_UDP, vxlan_header_l4_len);
> +	efx_gen_tun_header_udp(encap, sizeof(struct vxlanhdr));
> +	efx_gen_tun_header_vxlan(encap);
> +}
> +
> +#define geneve_header_l4_len	(sizeof(struct udphdr) + sizeof(struct genevehdr))
> +#define geneve4_header_len	(sizeof(struct ethhdr) + sizeof(struct iphdr) + geneve_header_l4_len)
> +static void efx_gen_geneve_header_ipv4(struct efx_tc_encap_action *encap)
> +{
> +	BUILD_BUG_ON(sizeof(encap->encap_hdr) < geneve4_header_len);
> +	efx_gen_tun_header_eth(encap, ETH_P_IP);
> +	efx_gen_tun_header_ipv4(encap, IPPROTO_UDP, geneve_header_l4_len);
> +	efx_gen_tun_header_udp(encap, sizeof(struct genevehdr));
> +	efx_gen_tun_header_geneve(encap);
> +}
> +
> +#ifdef CONFIG_IPV6
> +#define vxlan6_header_len	(sizeof(struct ethhdr) + sizeof(struct ipv6hdr) + vxlan_header_l4_len)
> +static void efx_gen_vxlan_header_ipv6(struct efx_tc_encap_action *encap)
> +{
> +	BUILD_BUG_ON(sizeof(encap->encap_hdr) < vxlan6_header_len);
> +	efx_gen_tun_header_eth(encap, ETH_P_IPV6);
> +	efx_gen_tun_header_ipv6(encap, IPPROTO_UDP, vxlan_header_l4_len);
> +	efx_gen_tun_header_udp(encap, sizeof(struct vxlanhdr));
> +	efx_gen_tun_header_vxlan(encap);
> +}
> +
> +#define geneve6_header_len	(sizeof(struct ethhdr) + sizeof(struct ipv6hdr) + geneve_header_l4_len)
> +static void efx_gen_geneve_header_ipv6(struct efx_tc_encap_action *encap)
> +{
> +	BUILD_BUG_ON(sizeof(encap->encap_hdr) < geneve6_header_len);
> +	efx_gen_tun_header_eth(encap, ETH_P_IPV6);
> +	efx_gen_tun_header_ipv6(encap, IPPROTO_UDP, geneve_header_l4_len);
> +	efx_gen_tun_header_udp(encap, sizeof(struct genevehdr));
> +	efx_gen_tun_header_geneve(encap);
> +}
> +#endif
> +
> +static void efx_gen_encap_header(struct efx_nic *efx,
> +				 struct efx_tc_encap_action *encap)
> +{
> +	encap->n_valid = encap->neigh->n_valid;
> +
> +	/* GCC stupidly thinks that only values explicitly listed in the enum
> +	 * definition can _possibly_ be sensible case values, so without this
> +	 * cast it complains about the IPv6 versions.
> +	 */
> +	switch ((int)encap->type) {
> +	case EFX_ENCAP_TYPE_VXLAN:
> +		efx_gen_vxlan_header_ipv4(encap);
> +		break;
> +	case EFX_ENCAP_TYPE_GENEVE:
> +		efx_gen_geneve_header_ipv4(encap);
> +		break;
> +#ifdef CONFIG_IPV6
> +	case EFX_ENCAP_TYPE_VXLAN | EFX_ENCAP_FLAG_IPV6:
> +		efx_gen_vxlan_header_ipv6(encap);
> +		break;
> +	case EFX_ENCAP_TYPE_GENEVE | EFX_ENCAP_FLAG_IPV6:
> +		efx_gen_geneve_header_ipv6(encap);
> +		break;
> +#endif
> +	default:
> +		/* unhandled encap type, can't happen */
> +		if (net_ratelimit())
> +			netif_err(efx, drv, efx->net_dev,
> +				  "Bogus encap type %d, can't generate\n",
> +				  encap->type);
> +
> +		/* Use fallback action. */
> +		encap->n_valid = false;
> +		break;
> +	}
>  }
>  
Hello Edward Cree,

Why do you need to refactor the efx_gen_encap_header function in the same series?
I saw that patch 5 defined this function, and patch 6 refactored it,
instead of writing it all at once?

Regards,
Hao Lan

>  static void efx_tc_update_encap(struct efx_nic *efx,
> @@ -278,14 +449,19 @@ static void efx_tc_update_encap(struct efx_nic *efx,
>  		}
>  	}
>  
> +	/* Make sure we don't leak arbitrary bytes on the wire;
> +	 * set an all-0s ethernet header.  A successful call to
> +	 * efx_gen_encap_header() will overwrite this.
> +	 */
> +	memset(encap->encap_hdr, 0, sizeof(encap->encap_hdr));
> +	encap->encap_hdr_len = ETH_HLEN;
> +
>  	if (encap->neigh) {
>  		read_lock_bh(&encap->neigh->lock);
> -		efx_gen_encap_header(encap);
> +		efx_gen_encap_header(efx, encap);
>  		read_unlock_bh(&encap->neigh->lock);
>  	} else {
>  		encap->n_valid = false;
> -		memset(encap->encap_hdr, 0, sizeof(encap->encap_hdr));
> -		encap->encap_hdr_len = ETH_HLEN;
>  	}
>  
>  	rc = efx_mae_update_encap_md(efx, encap);
> @@ -482,7 +658,7 @@ struct efx_tc_encap_action *efx_tc_flower_create_encap_md(
>  	}
>  	encap->dest_mport = rc;
>  	read_lock_bh(&encap->neigh->lock);
> -	efx_gen_encap_header(encap);
> +	efx_gen_encap_header(efx, encap);
>  	read_unlock_bh(&encap->neigh->lock);
>  
>  	rc = efx_mae_allocate_encap_md(efx, encap);
> 
> .
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ