inet6: resolve IPV6_TCLASS conversion problems

PATCH In review Gerrit Renker 3 participants 6 messages Aug 9, 2009 – Aug 13, 2009 1 message not in the archive

Subsystems (4)

Networking Network drivers IPv4/IPv6 UDP

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): resolve_IPV6_TCLASS

Gerrit Renker

author

2009-08-09 18:13 UTC #1 lore raw

cover inet6: resolve IPV6_TCLASS conversion problems

These are two patches dealing with the IPV6_TCLASS sticky socket option. They belong together, the second patch is a follow-up and fixes the remainder.

Patch #1: Removes unnecessary conversions (tclass being u8)
This is a resent, an OK was given in the reply of
http://marc.info/?l=linux-netdev&m=124820628409987&w=2
Patch #2: Solves the remaining problem of passing the socket option,

          fixing a bug (-1 became a non-default traffic class of 0xff).

Gerrit Renker

author

2009-08-09 18:13 UTC #2 lore raw

PATCH 1/2 inet6: Conversion from u8 to int

This replaces assignments of the type "int on LHS" = "u8 on RHS" with simpler code. The LHS can express all of the unsigned right hand side values, hence the assigned value can not be negative.

Signed-off-by Gerrit Renker <[email protected]>

---
 net/ipv6/icmp.c          |   17 ++++-------------
 net/ipv6/ip6_output.c    |   15 +++++----------
 net/ipv6/ipv6_sockglue.c |    2 --
 net/ipv6/raw.c           |    5 +----
 net/ipv6/udp.c           |    5 +----
 5 files changed, 11 insertions(+), 33 deletions(-)

net/ipv6/ipv6_sockglue.c +0 -2

@@ -1037,8 +1037,6 @@ static int do_ipv6_getsockopt(struct soc
 
 	case IPV6_TCLASS:
 		val = np->tclass;
-		if (val < 0)
-			val = 0;
 		break;
 
 	case IPV6_RECVTCLASS:

net/ipv6/raw.c +1 -4

@@ -877,11 +877,8 @@ static int rawv6_sendmsg(struct kiocb *i
 			hlimit = ip6_dst_hoplimit(dst);
 	}
 
-	if (tclass < 0) {
+	if (tclass < 0)
 		tclass = np->tclass;
-		if (tclass < 0)
-			tclass = 0;
-	}
 
 	if (msg->msg_flags&MSG_CONFIRM)
 		goto do_confirm;

net/ipv6/udp.c +1 -4

@@ -900,11 +900,8 @@ do_udp_sendmsg:
 			hlimit = ip6_dst_hoplimit(dst);
 	}
 
-	if (tclass < 0) {
+	if (tclass < 0)
 		tclass = np->tclass;
-		if (tclass < 0)
-			tclass = 0;
-	}
 
 	if (msg->msg_flags&MSG_CONFIRM)
 		goto do_confirm;

net/ipv6/icmp.c +4 ~~-13~~

@@ -323,7 +323,7 @@ void icmpv6_send(struct sk_buff *skb, u8
 	int iif = 0;
 	int addr_type = 0;
 	int len;
-	int hlimit, tclass;
+	int hlimit;
 	int err = 0;
 
 	if ((u8 *)hdr < skb->head ||

@@ -469,10 +469,6 @@ route_done:
 	if (hlimit < 0)
 		hlimit = ip6_dst_hoplimit(dst);
 
-	tclass = np->tclass;
-	if (tclass < 0)
-		tclass = 0;
-
 	msg.skb = skb;
 	msg.offset = skb_network_offset(skb);
 	msg.type = type;

@@ -488,8 +484,8 @@ route_done:
 
 	err = ip6_append_data(sk, icmpv6_getfrag, &msg,
 			      len + sizeof(struct icmp6hdr),
-			      sizeof(struct icmp6hdr),
-			      hlimit, tclass, NULL, &fl, (struct rt6_info*)dst,
+			      sizeof(struct icmp6hdr), hlimit,
+			      np->tclass, NULL, &fl, (struct rt6_info*)dst,
 			      MSG_DONTWAIT);
 	if (err) {
 		ip6_flush_pending_frames(sk);

@@ -522,7 +518,6 @@ static void icmpv6_echo_reply(struct sk_
 	struct dst_entry *dst;
 	int err = 0;
 	int hlimit;
-	int tclass;
 
 	saddr = &ipv6_hdr(skb)->daddr;

@@ -562,10 +557,6 @@ static void icmpv6_echo_reply(struct sk_
 	if (hlimit < 0)
 		hlimit = ip6_dst_hoplimit(dst);
 
-	tclass = np->tclass;
-	if (tclass < 0)
-		tclass = 0;
-
 	idev = in6_dev_get(skb->dev);
 
 	msg.skb = skb;

@@ -573,7 +564,7 @@ static void icmpv6_echo_reply(struct sk_
 	msg.type = ICMPV6_ECHO_REPLY;
 
 	err = ip6_append_data(sk, icmpv6_getfrag, &msg, skb->len + sizeof(struct icmp6hdr),
-				sizeof(struct icmp6hdr), hlimit, tclass, NULL, &fl,
+				sizeof(struct icmp6hdr), hlimit, np->tclass, NULL, &fl,
 				(struct rt6_info*)dst, MSG_DONTWAIT);
 
 	if (err) {

net/ipv6/ip6_output.c +5 ~~-10~~

@@ -206,7 +206,8 @@ int ip6_xmit(struct sock *sk, struct sk_
 	struct ipv6hdr *hdr;
 	u8  proto = fl->proto;
 	int seg_len = skb->len;
-	int hlimit, tclass;
+	int hlimit = -1;
+	int tclass = 0;
 	u32 mtu;
 
 	if (opt) {

@@ -249,19 +250,13 @@ int ip6_xmit(struct sock *sk, struct sk_
 	/*
 	 *	Fill in the IPv6 header
 	 */
-
-	hlimit = -1;
-	if (np)
+	if (np) {
+		tclass = np->tclass;
 		hlimit = np->hop_limit;
+	}
 	if (hlimit < 0)
 		hlimit = ip6_dst_hoplimit(dst);
 
-	tclass = -1;
-	if (np)
-		tclass = np->tclass;
-	if (tclass < 0)
-		tclass = 0;
-
 	*(__be32 *)hdr = htonl(0x60000000 | (tclass << 20)) | fl->fl6_flowlabel;
 
 	hdr->payload_len = htons(seg_len);

Gerrit Renker

author

2009-08-09 18:13 UTC #3 lore raw

PATCH 2/2 inet6: Set default traffic class

This patch addresses:
* assigning -1 to np->tclass as it is currently done is not very meaningful, since it turns into 0xff;
* RFC 3542, 6.5 allows -1 for clearing the sticky IPV6_TCLASS option and specifies -1 to mean "use kernel default":
- RFC 2460, 7. requires that the default traffic class must be zero for all 8 bits,
- this is consistent with RFC 2474, 4.1 which recommends a default PHB of 0, in combination with a value of the ECN field of "non-ECT" (RFC 3168, 5.).

This patch changes the meaning of -1 from assigning 255 to mean the RFC 2460 default, which at the same time allows to satisfy clearing the sticky TCLASS option as per RFC 3542, 6.5.

(When passing -1 as ancillary data, the fallback remains np->tclass, which
has either been set via socket options, or contains the default value.)

Signed-off-by Gerrit Renker <[email protected]>

---
 net/ipv6/ipv6_sockglue.c |    3 +++
 1 file changed, 3 insertions(+)

net/ipv6/ipv6_sockglue.c +3 -0

@@ -315,6 +315,9 @@ static int do_ipv6_setsockopt(struct soc
 			goto e_inval;
 		if (val < -1 || val > 0xff)
 			goto e_inval;
+		/* RFC 3542, 6.5: default traffic class of 0x0 */
+		if (val == -1)
+			val = 0;
 		np->tclass = val;
 		retv = 0;
 		break;

Rémi Denis-Courmont

2009-08-10 06:33 UTC #4 lore raw

Re: [Resent][PATCH 1/2] inet6: Conversion from u8 to int

In reply to Gerrit Renker

net/ipv6/ipv6_sockglue.c static int do_ipv6_getsockopt(struct soc · line 1040

Quoted 5 lines · On Sunday 09 August 2009 21:12:48 ext Gerrit Renker wrote:

net/ipv6/ipv6_sockglue.c +0 -2

 	case IPV6_TCLASS:
 		val = np->tclass;
-		if (val < 0)
-			val = 0;
 		break;

According to the IPv6 API RFC, if val is -1, the kernel shall pick the default value (which is zero). Are you sure this still works??

Signature · 2 lines

Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

ext Gerrit Renker

author

2009-08-10 07:08 UTC #5 lore raw

Re: [Resent][PATCH 1/2] inet6: Conversion from u8 to int

Quoting R?mi Denis-Courmont:

| On Sunday 09 August 2009 21:12:48 ext Gerrit Renker wrote:
| >  	case IPV6_TCLASS:
| >  		val = np->tclass;
| > -		if (val < 0)
| > -			val = 0;
| >  		break;
| 
| According to the IPv6 API RFC, if val is -1, the kernel shall pick the default 
| value (which is zero). Are you sure this still works??

Yes exactly that is the point, but it does not work since np->tclass is __u8, i.e. val will not be negative here.

What you refer to is done in the second patch and in the combination I think it will work, please have a look.

David Miller

maintainer

2009-08-13 23:28 UTC #6 lore raw

In reply to Gerrit Renker

From: Gerrit Renker <[email protected]>
Date: Sun, 9 Aug 2009 20:12:47 +0200

Quoted 8 lines

These are two patches dealing with the IPV6_TCLASS sticky socket option. They belong together, the second patch is a follow-up and fixes the remainder.
Patch #1: Removes unnecessary conversions (tclass being u8)
This is a resent, an OK was given in the reply of
http://marc.info/?l=linux-netdev&m=124820628409987&w=2
Patch #2: Solves the remaining problem of passing the socket option,
          fixing a bug (-1 became a non-default traffic class of 0xff).

Both applied to net-next-2.6