RE: [PATCH v2 net-next] consolidate duplicate code is skb_checksum_setup() helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 11 March 2014 13:56
> To: netdev@xxxxxxxxxxxxxxx
> Cc: Paul Durrant; davem@xxxxxxxxxxxxx; Eric Dumazet
> Subject: [PATCH v2 net-next] consolidate duplicate code is
> skb_checksum_setup() helpers
> 
> consolidate duplicate code is skb_checksum_setup() helpers
> 
> Realizing that the skb_maybe_pull_tail() calls in the IP-protocol
> specific portions of both helpers are terminal ones (i.e. no further
> pulls are expected), their maximum size to be pulled can be made match
> their minimal size needed, thus making the code identical and hence
> possible to be moved into another helper.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: David Miller <davem@xxxxxxxxxxxxx>
> Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
> ---
> v2: Use MAX_TCP_HDR_LEN. Use __sum16 instead of __be16.

Looks ok to me.

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
>  net/core/skbuff.c |  144 +++++++++++++++++++---------------------------------
> --
>  1 file changed, 52 insertions(+), 92 deletions(-)
> 
> --- 3.14-rc6/net/core/skbuff.c
> +++ 3.14-rc6-skb-checksum-setup-ip/net/core/skbuff.c
> @@ -3540,15 +3540,47 @@ static int skb_maybe_pull_tail(struct sk
>  	return 0;
>  }
> 
> +#define MAX_TCP_HDR_LEN (15 * 4)
> +
> +static __sum16 *skb_checksum_setup_ip(struct sk_buff *skb,
> +				      typeof(IPPROTO_IP) proto,
> +				      unsigned int off)
> +{
> +	switch (proto) {
> +		int err;
> +
> +	case IPPROTO_TCP:
> +		err = skb_maybe_pull_tail(skb, off + sizeof(struct tcphdr),
> +					  off + MAX_TCP_HDR_LEN);
> +		if (!err && !skb_partial_csum_set(skb, off,
> +						  offsetof(struct tcphdr,
> +							   check)))
> +			err = -EPROTO;
> +		return err ? ERR_PTR(err) : &tcp_hdr(skb)->check;
> +
> +	case IPPROTO_UDP:
> +		err = skb_maybe_pull_tail(skb, off + sizeof(struct udphdr),
> +					  off + sizeof(struct udphdr));
> +		if (!err && !skb_partial_csum_set(skb, off,
> +						  offsetof(struct udphdr,
> +							   check)))
> +			err = -EPROTO;
> +		return err ? ERR_PTR(err) : &udp_hdr(skb)->check;
> +	}
> +
> +	return ERR_PTR(-EPROTO);
> +}
> +
>  /* This value should be large enough to cover a tagged ethernet header plus
>   * maximally sized IP and TCP or UDP headers.
>   */
>  #define MAX_IP_HDR_LEN 128
> 
> -static int skb_checksum_setup_ip(struct sk_buff *skb, bool recalculate)
> +static int skb_checksum_setup_ipv4(struct sk_buff *skb, bool recalculate)
>  {
>  	unsigned int off;
>  	bool fragment;
> +	__sum16 *csum;
>  	int err;
> 
>  	fragment = false;
> @@ -3569,51 +3601,15 @@ static int skb_checksum_setup_ip(struct
>  	if (fragment)
>  		goto out;
> 
> -	switch (ip_hdr(skb)->protocol) {
> -	case IPPROTO_TCP:
> -		err = skb_maybe_pull_tail(skb,
> -					  off + sizeof(struct tcphdr),
> -					  MAX_IP_HDR_LEN);
> -		if (err < 0)
> -			goto out;
> -
> -		if (!skb_partial_csum_set(skb, off,
> -					  offsetof(struct tcphdr, check))) {
> -			err = -EPROTO;
> -			goto out;
> -		}
> -
> -		if (recalculate)
> -			tcp_hdr(skb)->check =
> -				~csum_tcpudp_magic(ip_hdr(skb)->saddr,
> -						   ip_hdr(skb)->daddr,
> -						   skb->len - off,
> -						   IPPROTO_TCP, 0);
> -		break;
> -	case IPPROTO_UDP:
> -		err = skb_maybe_pull_tail(skb,
> -					  off + sizeof(struct udphdr),
> -					  MAX_IP_HDR_LEN);
> -		if (err < 0)
> -			goto out;
> -
> -		if (!skb_partial_csum_set(skb, off,
> -					  offsetof(struct udphdr, check))) {
> -			err = -EPROTO;
> -			goto out;
> -		}
> -
> -		if (recalculate)
> -			udp_hdr(skb)->check =
> -				~csum_tcpudp_magic(ip_hdr(skb)->saddr,
> -						   ip_hdr(skb)->daddr,
> -						   skb->len - off,
> -						   IPPROTO_UDP, 0);
> -		break;
> -	default:
> -		goto out;
> -	}
> -
> +	csum = skb_checksum_setup_ip(skb, ip_hdr(skb)->protocol, off);
> +	if (IS_ERR(csum))
> +		return PTR_ERR(csum);
> +
> +	if (recalculate)
> +		*csum = ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
> +					   ip_hdr(skb)->daddr,
> +					   skb->len - off,
> +					   ip_hdr(skb)->protocol, 0);
>  	err = 0;
> 
>  out:
> @@ -3636,6 +3632,7 @@ static int skb_checksum_setup_ipv6(struc
>  	unsigned int len;
>  	bool fragment;
>  	bool done;
> +	__sum16 *csum;
> 
>  	fragment = false;
>  	done = false;
> @@ -3713,51 +3710,14 @@ static int skb_checksum_setup_ipv6(struc
>  	if (!done || fragment)
>  		goto out;
> 
> -	switch (nexthdr) {
> -	case IPPROTO_TCP:
> -		err = skb_maybe_pull_tail(skb,
> -					  off + sizeof(struct tcphdr),
> -					  MAX_IPV6_HDR_LEN);
> -		if (err < 0)
> -			goto out;
> -
> -		if (!skb_partial_csum_set(skb, off,
> -					  offsetof(struct tcphdr, check))) {
> -			err = -EPROTO;
> -			goto out;
> -		}
> -
> -		if (recalculate)
> -			tcp_hdr(skb)->check =
> -				~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> -						 &ipv6_hdr(skb)->daddr,
> -						 skb->len - off,
> -						 IPPROTO_TCP, 0);
> -		break;
> -	case IPPROTO_UDP:
> -		err = skb_maybe_pull_tail(skb,
> -					  off + sizeof(struct udphdr),
> -					  MAX_IPV6_HDR_LEN);
> -		if (err < 0)
> -			goto out;
> -
> -		if (!skb_partial_csum_set(skb, off,
> -					  offsetof(struct udphdr, check))) {
> -			err = -EPROTO;
> -			goto out;
> -		}
> -
> -		if (recalculate)
> -			udp_hdr(skb)->check =
> -				~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> -						 &ipv6_hdr(skb)->daddr,
> -						 skb->len - off,
> -						 IPPROTO_UDP, 0);
> -		break;
> -	default:
> -		goto out;
> -	}
> -
> +	csum = skb_checksum_setup_ip(skb, nexthdr, off);
> +	if (IS_ERR(csum))
> +		return PTR_ERR(csum);
> +
> +	if (recalculate)
> +		*csum = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> +					 &ipv6_hdr(skb)->daddr,
> +					 skb->len - off, nexthdr, 0);
>  	err = 0;
> 
>  out:
> @@ -3775,7 +3735,7 @@ int skb_checksum_setup(struct sk_buff *s
> 
>  	switch (skb->protocol) {
>  	case htons(ETH_P_IP):
> -		err = skb_checksum_setup_ip(skb, recalculate);
> +		err = skb_checksum_setup_ipv4(skb, recalculate);
>  		break;
> 
>  	case htons(ETH_P_IPV6):
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Discussion]     [TCP Instrumentation]     [Ethernet Bridging]     [Linux Wireless Networking]     [Linux WPAN Networking]     [Linux Host AP]     [Linux WPAN Networking]     [Linux Bluetooth Networking]     [Linux ATH6KL Networking]     [Linux Networking Users]     [Linux Coverity]     [VLAN]     [Git]     [IETF Annouce]     [Linux Assembly]     [Security]     [Bugtraq]     [Yosemite Information]     [MIPS Linux]     [ARM Linux Kernel]     [ARM Linux]     [Linux Virtualization]     [Linux IDE]     [Linux RAID]     [Linux SCSI]