netdev - Re: [net-next-2.6 PATCH 2/2] net: replace ipfragok with skb->local

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-next>] [day] [month] [year] [list]

Message-ID: <4BC7CAC1.4000803@cn.fujitsu.com>
Date:	Fri, 16 Apr 2010 10:26:09 +0800
From:	Shan Wei <shanwei@...fujitsu.com>
To:	Herbert Xu <herbert@...dor.hengli.com.au>
CC:	David Miller <davem@...emloft.net>, yinghai.lu@...cle.com,
	kuznet@....inr.ac.ru, pekkas@...core.fi, jmorris@...ei.org,
	"yoshfuji@...ux-ipv6.org >> YOSHIFUJI Hideaki" 
	<yoshfuji@...ux-ipv6.org>, Patrick McHardy <kaber@...sh.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	dccp@...r.kernel.org, linux-sctp@...r.kernel.org,
	kleptog@...na.org, jchapman@...alix.com, mostrows@...akeasy.net,
	acme@...nectiva.com.br
Subject: Re: [net-next-2.6 PATCH 2/2] net: replace ipfragok with	skb->local_df

Herbert Xu wrote, at 04/15/2010 11:19 PM:
> On Thu, Apr 15, 2010 at 09:04:49PM +0800, Shan Wei wrote:
>> As Herbert Xu said: we should be able to simply replace ipfragok
>> with skb->local_df. commit f88037(sctp: Drop ipfargok in sctp_xmit function)
>> has droped ipfragok and set local_df value properly.
>>
>> The patch kills the ipfragok parameter of .queue_xmit().
> 
> Both patches look good to me.
> 
>> @@ -370,7 +370,7 @@ packet_routed:
>>  	skb_reset_network_header(skb);
>>  	iph = ip_hdr(skb);
>>  	*((__be16 *)iph) = htons((4 << 12) | (5 << 8) | (inet->tos & 0xff));
>> -	if (ip_dont_fragment(sk, &rt->u.dst) && !ipfragok)
>> +	if (ip_dont_fragment(sk, &rt->u.dst) && !skb->local_df)
>>  		iph->frag_off = htons(IP_DF);
>>  	else
>>  		iph->frag_off = 0;
> 
> This hunk looked suspecious at first.  However, it is OK because
> ever calls this with ipfragok == 1, or local_df == 1.
> 
> The first is obvious from this patch itself, the second not quite
> so obvious.
> 
> As nobody calls it with local_df == 1 anyway, and strictly speaking
> local_df shouldn't be set at all by the caller of ip_queue_xmit, we
> should simply remove the && ... bit and have 
> 
> 	if (ip_dont_fragment(sk, &rt->u.dst))

Now, PPPoX/PPPoL2TP driver still use ip_queue_xmit to send packets with ipfragok == 1.
So, now we can't remove the && ... bit. 


-- 
Best Regards
-----
Shan Wei

> 
> Cheers,


--
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