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: <9AAE0902D5BC7E449B7C8E4E778ABCD028296C@AMSPEX01CL01.citrite.net>
Date:	Wed, 12 Mar 2014 09:46:09 +0000
From:	Paul Durrant <Paul.Durrant@...rix.com>
To:	Jan Beulich <JBeulich@...e.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>
Subject: RE: [PATCH v2 net-next] consolidate duplicate code is
 skb_checksum_setup() helpers

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@...e.com]
> Sent: 11 March 2014 13:56
> To: netdev@...r.kernel.org
> Cc: Paul Durrant; davem@...emloft.net; 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@...e.com>
> Cc: Paul Durrant <paul.durrant@...rix.com>
> Cc: David Miller <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> ---
> v2: Use MAX_TCP_HDR_LEN. Use __sum16 instead of __be16.

Looks ok to me.

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

> ---
>  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ