[PATCH net-next v3 1/4] ipv6: remove old conditions on flow label sharing

[PATCH net-next v3 1/4] ipv6: remove old conditions on flow label sharing

[Florent Fourcot]

The code of flow label in Linux Kernel follows
the rules of RFC 1809 (an informational one) for
conditions on flow label sharing. There rules are
not in the last proposed standard for flow label
(RFC 6437), or in the previous one (RFC 3697).

Since this code does not follow any current or
old standard, we can remove it.

With this removal, the ipv6_opt_cmp and
pv6_hdr_cmp function are now a dead code and it
can be removed too.

Changelog of v2:
 * add justification for the change
 * remove the condition on IPv6 options
 * remove ipv6_opt_cmp
v3:
 * remove ipv6_hdr_cmp

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
---
 net/ipv6/ip6_flowlabel.c | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 46e8843..819578e 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -465,34 +465,6 @@ static int mem_check(struct sock *sk)
 	return 0;
 }
 
-static bool ipv6_hdr_cmp(struct ipv6_opt_hdr *h1, struct ipv6_opt_hdr *h2)
-{
-	if (h1 == h2)
-		return false;
-	if (h1 == NULL || h2 == NULL)
-		return true;
-	if (h1->hdrlen != h2->hdrlen)
-		return true;
-	return memcmp(h1+1, h2+1, ((h1->hdrlen+1)<<3) - sizeof(*h1));
-}
-
-static bool ipv6_opt_cmp(struct ipv6_txoptions *o1, struct ipv6_txoptions *o2)
-{
-	if (o1 == o2)
-		return false;
-	if (o1 == NULL || o2 == NULL)
-		return true;
-	if (o1->opt_nflen != o2->opt_nflen)
-		return true;
-	if (ipv6_hdr_cmp(o1->hopopt, o2->hopopt))
-		return true;
-	if (ipv6_hdr_cmp(o1->dst0opt, o2->dst0opt))
-		return true;
-	if (ipv6_hdr_cmp((struct ipv6_opt_hdr *)o1->srcrt, (struct ipv6_opt_hdr *)o2->srcrt))
-		return true;
-	return false;
-}
-
 static inline void fl_link(struct ipv6_pinfo *np, struct ipv6_fl_socklist *sfl,
 		struct ip6_flowlabel *fl)
 {
@@ -603,11 +575,6 @@ recheck:
 				     uid_eq(fl1->owner.uid, fl->owner.uid)))
 					goto release;
 
-				err = -EINVAL;
-				if (!ipv6_addr_equal(&fl1->dst, &fl->dst) ||
-				    ipv6_opt_cmp(fl1->opt, fl->opt))
-					goto release;
-
 				err = -ENOMEM;
 				if (sfl1 == NULL)
 					goto release;
-- 
1.8.4.rc3

[PATCH net-next v2 2/4] ipv6: enable IPV6_FLOWLABEL_MGR for getsockopt

[Florent Fourcot]

It is already possible to set/put/renew a label
with IPV6_FLOWLABEL_MGR and setsockopt. This patch
add the possibility to get information about this
label (current value, time before expiration, etc).

It helps application to take decision for a renew
or a release of the label.

v2:
 * Add spin_lock to prevent race condition
 * return -ENOENT if no result found
 * check if flr_action is GET

Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
---
 include/net/ipv6.h       |  1 +
 net/ipv6/ip6_flowlabel.c | 27 +++++++++++++++++++++++++++
 net/ipv6/ipv6_sockglue.c | 28 ++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index dd96638..2a5f668 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -250,6 +250,7 @@ struct ipv6_txoptions *fl6_merge_options(struct ipv6_txoptions *opt_space,
 					 struct ipv6_txoptions *fopt);
 void fl6_free_socklist(struct sock *sk);
 int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen);
+int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq);
 int ip6_flowlabel_init(void);
 void ip6_flowlabel_cleanup(void);
 
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 819578e..76e62a1 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -475,6 +475,33 @@ static inline void fl_link(struct ipv6_pinfo *np, struct ipv6_fl_socklist *sfl,
 	spin_unlock_bh(&ip6_sk_fl_lock);
 }
 
+int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq)
+{
+	struct ipv6_pinfo *np = inet6_sk(sk);
+	struct ipv6_fl_socklist *sfl;
+
+	spin_lock_bh(&ip6_fl_lock);
+	rcu_read_lock_bh();
+
+	for_each_sk_fl_rcu(np, sfl) {
+		if (sfl->fl->label == (np->flow_label & IPV6_FLOWLABEL_MASK)) {
+			freq->flr_label = sfl->fl->label;
+			freq->flr_dst = sfl->fl->dst;
+			freq->flr_share = sfl->fl->share;
+			freq->flr_expires = (sfl->fl->expires - jiffies) / HZ;
+			freq->flr_linger = sfl->fl->linger / HZ;
+
+			rcu_read_unlock_bh();
+			spin_unlock_bh(&ip6_fl_lock);
+			return 0;
+		}
+	}
+	rcu_read_unlock_bh();
+	spin_unlock_bh(&ip6_fl_lock);
+
+	return -ENOENT;
+}
+
 int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen)
 {
 	int uninitialized_var(err);
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 4919a8e..1c6ce31 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -1212,6 +1212,34 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		val = np->sndflow;
 		break;
 
+	case IPV6_FLOWLABEL_MGR:
+	{
+		struct in6_flowlabel_req freq;
+
+		if (len < sizeof(freq))
+			return -EINVAL;
+
+		if (copy_from_user(&freq, optval, sizeof(freq)))
+			return -EFAULT;
+
+		if (freq.flr_action != IPV6_FL_A_GET)
+			return -EINVAL;
+
+		len = sizeof(freq);
+		memset(&freq, 0, sizeof(freq));
+
+		val = ipv6_flowlabel_opt_get(sk, &freq);
+		if (val < 0)
+			return val;
+
+		if (put_user(len, optlen))
+			return -EFAULT;
+		if (copy_to_user(optval, &freq, len))
+			return -EFAULT;
+
+		return 0;
+	}
+
 	case IPV6_ADDR_PREFERENCES:
 		val = 0;
 
-- 
1.8.4.rc3

Re: [PATCH net-next v2 2/4] ipv6: enable IPV6_FLOWLABEL_MGR for getsockopt

[Hannes Frederic Sowa]

On Tue, Nov 05, 2013 at 03:28:56PM +0100, Florent Fourcot wrote:
> +	spin_lock_bh(&ip6_fl_lock);
> +	rcu_read_lock_bh();
> +
> +	for_each_sk_fl_rcu(np, sfl) {
> +		if (sfl->fl->label == (np->flow_label & IPV6_FLOWLABEL_MASK)) {

The iteration is protected by rcu, so need to take the ip6_fl_lock. We should
lock the smallest region which needs to be protected. This is only the body of
the if.

Using fl->label without lock is fine, because it is immutable after interning.

> +			freq->flr_label = sfl->fl->label;
> +			freq->flr_dst = sfl->fl->dst;
> +			freq->flr_share = sfl->fl->share;
> +			freq->flr_expires = (sfl->fl->expires - jiffies) / HZ;
> +			freq->flr_linger = sfl->fl->linger / HZ;
> +
> +			rcu_read_unlock_bh();
> +			spin_unlock_bh(&ip6_fl_lock);

Please reverse these two lines then.

Could you take a look at your patch regarding fl6_renew, too? Can we
push locking into the function there, too?

Greetings,

  Hannes

Re: [PATCH net-next v2 2/4] ipv6: enable IPV6_FLOWLABEL_MGR for getsockopt

[Hannes Frederic Sowa]

On Tue, Nov 05, 2013 at 05:08:38PM +0100, Hannes Frederic Sowa wrote:
> On Tue, Nov 05, 2013 at 03:28:56PM +0100, Florent Fourcot wrote:
> > +	spin_lock_bh(&ip6_fl_lock);
> > +	rcu_read_lock_bh();
> > +
> > +	for_each_sk_fl_rcu(np, sfl) {
> > +		if (sfl->fl->label == (np->flow_label & IPV6_FLOWLABEL_MASK)) {
> 
> The iteration is protected by rcu, so need to take the ip6_fl_lock. We should
                                        ^no                         ^ here.

Sorry,

  Hannes

[PATCH net-next 3/4] ipv6: increase maximum lifetime of flow labels

[Florent Fourcot]

If the last RFC 6437 does not give any constraints
for lifetime of flow labels, the previous RFC 3697
spoke of a minimum of 120 seconds quarantine
for a flow label.

The maximum linger is currently set to 60 seconds
and does not allow this configuration without
CAP_NET_ADMIN right.

This patch increases the maximum linger to 150
seconds, allowing more flexibility to standard
users.

Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
---
 net/ipv6/ip6_flowlabel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 76e62a1..41ced9c 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -41,7 +41,7 @@
 #define FL_MIN_LINGER	6	/* Minimal linger. It is set to 6sec specified
 				   in old IPv6 RFC. Well, it was reasonable value.
 				 */
-#define FL_MAX_LINGER	60	/* Maximal linger timeout */
+#define FL_MAX_LINGER	150	/* Maximal linger timeout */
 
 /* FL hash table */
 
-- 
1.8.4.rc3

[PATCH net-next 4/4] ipv6: protect flow label renew against GC

[Florent Fourcot]

Take ip6_fl_lock before to read and update a
label. It prevents race condition if GC is
running.

Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
---
 net/ipv6/ip6_flowlabel.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 41ced9c..1d2fc48 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -540,11 +540,13 @@ int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen)
 		return -ESRCH;
 
 	case IPV6_FL_A_RENEW:
+		spin_lock_bh(&ip6_fl_lock);
 		rcu_read_lock_bh();
 		for_each_sk_fl_rcu(np, sfl) {
 			if (sfl->fl->label == freq.flr_label) {
 				err = fl6_renew(sfl->fl, freq.flr_linger, freq.flr_expires);
 				rcu_read_unlock_bh();
+				spin_unlock_bh(&ip6_fl_lock);
 				return err;
 			}
 		}
@@ -555,10 +557,12 @@ int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen)
 			fl = fl_lookup(net, freq.flr_label);
 			if (fl) {
 				err = fl6_renew(fl, freq.flr_linger, freq.flr_expires);
+				spin_unlock_bh(&ip6_fl_lock);
 				fl_release(fl);
 				return err;
 			}
 		}
+		spin_unlock_bh(&ip6_fl_lock);
 		return -ESRCH;
 
 	case IPV6_FL_A_GET:
-- 
1.8.4.rc3