From: Eric Dumazet <eric.dumazet@gmail.com>
To: Stefanos Harhalakis <v13@v13.gr>, David Miller <davem@davemloft.net>
Cc: netdev <netdev@vger.kernel.org>
Subject: [PATCH net-next-2.6] ipv4: more compliant RFC 3168 support
Date: Sun, 15 May 2011 15:35:42 +0200 [thread overview]
Message-ID: <1305466542.3120.129.camel@edumazet-laptop> (raw)
In-Reply-To: <1305464176.3120.113.camel@edumazet-laptop>
Commit 6623e3b24a5e (ipv4: IP defragmentation must be ECN aware) was an
attempt to not lose "Congestion Experienced" (CE) indications when
performing datagram defragmentation.
Stefanos Harhalakis raised the point that RFC 3168 requirements were not
completely met by this commit.
In particular, we MUST detect invalid combinations and eventually drop
illegal frames.
Reported-by: Stefanos Harhalakis <v13@v13.gr>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/ip_fragment.c | 66 ++++++++++++++++++++++++++-------------
1 file changed, 44 insertions(+), 22 deletions(-)
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index b1d282f..1f74089 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -77,20 +77,46 @@ struct ipq {
struct inet_peer *peer;
};
-#define IPFRAG_ECN_CLEAR 0x01 /* one frag had INET_ECN_NOT_ECT */
-#define IPFRAG_ECN_SET_CE 0x04 /* one frag had INET_ECN_CE */
+/* RFC 3168 support :
+ * We want to check ECN values of all fragments, do detect invalid combinations.
+ * In ipq->ecn, we store the OR value of each ip4_frag_ecn() fragment value.
+ */
+enum {
+ IPFRAG_ECN_NOT_ECT = 0x01, /* one frag had ECN_NOT_ECT */
+ IPFRAG_ECN_ECT_1 = 0x02, /* one frag had ECN_ECT_1 */
+ IPFRAG_ECN_ECT_0 = 0x04, /* one frag had ECN_ECT_0 */
+ IPFRAG_ECN_CE = 0x08, /* one frag had ECN_CE */
+};
static inline u8 ip4_frag_ecn(u8 tos)
{
- tos = (tos & INET_ECN_MASK) + 1;
- /*
- * After the last operation we have (in binary):
- * INET_ECN_NOT_ECT => 001
- * INET_ECN_ECT_1 => 010
- * INET_ECN_ECT_0 => 011
- * INET_ECN_CE => 100
- */
- return (tos & 2) ? 0 : tos;
+ return 1 << (tos & INET_ECN_MASK);
+}
+
+/* Given the OR values of all fragments, apply RFC 3168 5.3 requirements
+ * Return : -1 if frame should be dropped.
+ * >= 0 value, to be ORed in to final iph->tos field
+ */
+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;
+ }
}
static struct inet_frags ip4_frags;
@@ -524,9 +550,15 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
int len;
int ihlen;
int err;
+ int ecn;
ipq_kill(qp);
+ ecn = ip4_frag_ecn_fold(qp->ecn);
+ if (unlikely(ecn == -1)) {
+ err = -EINVAL;
+ goto out_fail;
+ }
/* Make the one we just received the head. */
if (prev) {
head = prev->next;
@@ -605,17 +637,7 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
iph = ip_hdr(head);
iph->frag_off = 0;
iph->tot_len = htons(len);
- /* RFC3168 5.3 Fragmentation support
- * If one fragment had INET_ECN_NOT_ECT,
- * reassembled frame also has INET_ECN_NOT_ECT
- * Elif one fragment had INET_ECN_CE
- * reassembled frame also has INET_ECN_CE
- */
- if (qp->ecn & IPFRAG_ECN_CLEAR)
- iph->tos &= ~INET_ECN_MASK;
- else if (qp->ecn & IPFRAG_ECN_SET_CE)
- iph->tos |= INET_ECN_CE;
-
+ iph->tos |= ecn;
IP_INC_STATS_BH(net, IPSTATS_MIB_REASMOKS);
qp->q.fragments = NULL;
qp->q.fragments_tail = NULL;
next parent reply other threads:[~2011-05-15 13:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <201105141938.28344.v13@v13.gr>
[not found] ` <1305393402.3120.67.camel@edumazet-laptop>
[not found] ` <1305393711.3120.69.camel@edumazet-laptop>
[not found] ` <201105151337.22831.v13@v13.gr>
[not found] ` <1305461344.3120.112.camel@edumazet-laptop>
[not found] ` <1305464176.3120.113.camel@edumazet-laptop>
2011-05-15 13:35 ` Eric Dumazet [this message]
2011-05-15 15:08 ` [PATCH net-next-2.6] ipv4: more compliant RFC 3168 support Stefanos Harhalakis
2011-05-15 16:01 ` Eric Dumazet
2011-05-16 17:16 ` Eric Dumazet
2011-05-16 18:37 ` [PATCH net-next-2.6 v2] " Eric Dumazet
2011-05-16 18:49 ` David Miller
2011-05-16 19:17 ` Joe Perches
2011-05-16 21:33 ` [PATCH net-next-2.6] " Hagen Paul Pfeifer
2011-05-16 21:38 ` Eric Dumazet
2011-05-16 21:49 ` Hagen Paul Pfeifer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1305466542.3120.129.camel@edumazet-laptop \
--to=eric.dumazet@gmail.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=v13@v13.gr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox