diff mbox

[net-next,07/13] ipv6: Change "final" protocol processing for encapsulation

Message ID 1462985253-2380625-8-git-send-email-tom@herbertland.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Tom Herbert May 11, 2016, 4:47 p.m. UTC
When performing foo-over-UDP, UDP packets are processed by the
encapsulation handler which returns another protocol to process.
This may result in processing two (or more) protocols in the
loop that are marked as INET6_PROTO_FINAL. The actions taken
for hitting a final protocol, in particular the skb_postpull_rcsum
can only be performed once.

This patch set adds a check of a final protocol has been seen. The
rules are:
  - If the final protocol has not been seen any protocol is processed
    (final and non-final). In the case of a final protocol, the final
    actions are taken (like the skb_postpull_rcsum)
  - If a final protocol has been seen (e.g. an encapsulating UDP
    header) then no further non-final protocols are allowed
    (e.g. extension headers). For more final protocols the
    final actions are not taken (e.g. skb_postpull_rcsum).

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/ipv6/ip6_input.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Shmulik Ladkani May 13, 2016, 6:51 a.m. UTC | #1
Hi,

On Wed, 11 May 2016 09:47:27 -0700 Tom Herbert <tom@herbertland.com> wrote:
> When performing foo-over-UDP, UDP packets are processed by the
> encapsulation handler which returns another protocol to process.
> This may result in processing two (or more) protocols in the
> loop that are marked as INET6_PROTO_FINAL. The actions taken
> for hitting a final protocol, in particular the skb_postpull_rcsum
> can only be performed once.
> 
> This patch set adds a check of a final protocol has been seen. The
> rules are:
>   - If the final protocol has not been seen any protocol is processed
>     (final and non-final). In the case of a final protocol, the final
>     actions are taken (like the skb_postpull_rcsum)
>   - If a final protocol has been seen (e.g. an encapsulating UDP
>     header) then no further non-final protocols are allowed
>     (e.g. extension headers). For more final protocols the
>     final actions are not taken (e.g. skb_postpull_rcsum).
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>

Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Out of curiousity, you execute the "final actions" on the first
INET6_PROTO_FINAL occurence. Would it make any sense to defer the
actions and do them upon the last INET6_PROTO_FINAL occurence?
Or must them get executed once we encounter the first INET6_PROTO_FINAL
proto handler?

Also, just a nit, seems better if commit title is
  'Ensure "final" protocol processing is performed once'

Thanks
Shmulik
diff mbox

Patch

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 2a0258a..7d98d01 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -216,6 +216,7 @@  static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk
 	unsigned int nhoff;
 	int nexthdr;
 	bool raw;
+	bool have_final = false;
 
 	/*
 	 *	Parse extension headers
@@ -235,9 +236,21 @@  resubmit:
 	if (ipprot) {
 		int ret;
 
-		if (ipprot->flags & INET6_PROTO_FINAL) {
+		if (have_final) {
+			if (!(ipprot->flags & INET6_PROTO_FINAL)) {
+				/* Once we've seen a final protocol don't
+				 * allow encapsulation on any non-final
+				 * ones. This allows foo in UDP encapsulation
+				 * to work.
+				 */
+				goto discard;
+			}
+		} else if (ipprot->flags & INET6_PROTO_FINAL) {
 			const struct ipv6hdr *hdr;
 
+			/* Only do this once for first final protocol */
+			have_final = true;
+
 			/* Free reference early: we don't need it any more,
 			   and it may hold ip_conntrack module loaded
 			   indefinitely. */