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

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

[David Miller]

Your netdev entry on the CC: list was literally:

	" <netdev@vger.kernel.org>,

which caused your patch to not make it netdev and therefore
it also didn't make it into patchwork.

Please resubmit your patches (both of them) with the CC:
list fixed up.

Thank you.

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

[Shan Wei]

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,

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

[Herbert Xu]

On Fri, Apr 16, 2010 at 10:26:09AM +0800, Shan Wei wrote:
> 
> Now, PPPoX/PPPoL2TP driver still use ip_queue_xmit to send packets with ipfragok == 1.
> So, now we can't remove the && ... bit. 

Huh? If they still call ip_queue_xmit with ipfragok then surely
the build will fail after your patch as it removes the ipfragok
argument?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Herbert Xu]

On Fri, Apr 16, 2010 at 02:43:44PM +0800, Herbert Xu wrote:
> On Fri, Apr 16, 2010 at 10:26:09AM +0800, Shan Wei wrote:
> > 
> > Now, PPPoX/PPPoL2TP driver still use ip_queue_xmit to send packets with ipfragok == 1.
> > So, now we can't remove the && ... bit. 
> 
> Huh? If they still call ip_queue_xmit with ipfragok then surely
> the build will fail after your patch as it removes the ipfragok
> argument?

Nevermind, I was looking at the wrong tree.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt