netdev - Re: [PATCH net-next-2.6] ipv4: more compliant RFC 3168 support

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: <1305475310.3120.146.camel@edumazet-laptop>
Date:	Sun, 15 May 2011 18:01:50 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Stefanos Harhalakis <v13@....gr>
Cc:	David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next-2.6] ipv4: more compliant RFC 3168 support

Le dimanche 15 mai 2011 à 18:08 +0300, Stefanos Harhalakis a écrit :
> Hello,
> 
> On Sunday 15 of May 2011, Eric Dumazet wrote:
> > +static inline int ip4_frag_ecn_fold(u8 ecn)
> > +{
> > +	switch (ecn) {
> > +	/* If same ECN combination was observed on all fragments, do nothing */
> > +	case IPFRAG_ECN_NOT_ECT:
> > +	case IPFRAG_ECN_ECT_1:
> > +	case IPFRAG_ECN_ECT_0:
> > +	case IPFRAG_ECN_CE:
> > +	/* if a ECT_1 ECT_0 combination was observed, do nothing as well */
> > +	case IPFRAG_ECN_ECT_0 | IPFRAG_ECN_ECT_1:
> > +		return 0;
> > +	/* at least one fragment had CE, and others ECT_0 or ECT_1 */
> > +	case IPFRAG_ECN_CE | IPFRAG_ECN_ECT_0:
> > +	case IPFRAG_ECN_CE | IPFRAG_ECN_ECT_1:
> > +	case IPFRAG_ECN_CE | IPFRAG_ECN_ECT_0 | IPFRAG_ECN_ECT_1:
> > +		return INET_ECN_CE;
> > +	/* other combinations are invalid : drop frame */
> > +	default:
> > +		return -1;
> > +	}
> >  }
> 
> You may wish to simplify this exhaustive check to:
> 
> if (unlikely((ecn & IPFRAG_ECN_NOT_ECT) && ecn!=IPFRAG_ECN_NOT_ECT))
>   return -1;
> else if (ecn & IPFRAG_ECN_CE)
>   return INET_ECN_CE;
> else
>   return 0;
> 
> although I'm not sure which method will be faster.
> 

Problem of this version is that common frames in the Internet (NOT_ECT
or ECT_X or ECT_X) will take the longest path to come to "return 0;"

a switch() version is fast because gcc emits a table based jump

> Also, returning the exact same value for NOT_ECT and ECT_X and then ORing
> this with the TOS seems like it will make it loose the ECT_X info. No? (but
> also, I'm not sure if this is needed anyway from that point on).
> 

I dont understand what you mean here. We really need to not loose ECT_X,
and I believe we dont.

-1 : Drop the frame anyway
0 : No change on iph->tos field (we keep its value. it can have ECT_X.
Remember all fragments share same (iph->tos & 3) value)
3 : We make sure iph->tos is ORed with 3 to assert CE on result frame.

> p.s. I'm not sure whether this message will make it to the netdev list.

It should, no worry.


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