Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2 7/7] ip/xfrm: Improve error strings
From: David Ward @ 2013-03-25 14:23 UTC (permalink / raw)
  To: netdev; +Cc: David Ward
In-Reply-To: <1364221399-1024-1-git-send-email-david.ward@ll.mit.edu>

Quotation marks are now used only to indicate literal text on the
command line.

Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 ip/ipxfrm.c      |   64 +++++++++++++++++++++++++++---------------------------
 ip/xfrm_policy.c |   28 +++++++++++-----------
 ip/xfrm_state.c  |   40 ++++++++++++++++----------------
 3 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 3113573..0495ff4 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -660,7 +660,7 @@ int xfrm_parse_mark(struct xfrm_mark *mark, int *argcp, char ***argvp)
 
 	NEXT_ARG();
 	if (get_u32(&mark->v, *argv, 0)) {
-		invarg("Illegal \"mark\" value\n", *argv);
+		invarg("MARK value is invalid\n", *argv);
 	}
 	if (argc > 1)
 		NEXT_ARG();
@@ -672,7 +672,7 @@ int xfrm_parse_mark(struct xfrm_mark *mark, int *argcp, char ***argvp)
 	if (strcmp(*argv, "mask") == 0) {
 		NEXT_ARG();
 		if (get_u32(&mark->m, *argv, 0)) {
-			invarg("Illegal \"mark\" mask\n", *argv);
+			invarg("MASK value is invalid\n", *argv);
 		}
 	} else {
 		mark->m = 0xffffffff;
@@ -1010,7 +1010,7 @@ int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 
 			get_prefix(&src, *argv, preferred_family);
 			if (src.family == AF_UNSPEC)
-				invarg("\"src\" address family is AF_UNSPEC", *argv);
+				invarg("value after \"src\" has an unrecognized address family", *argv);
 			if (family)
 				*family = src.family;
 
@@ -1023,7 +1023,7 @@ int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 
 			get_prefix(&dst, *argv, preferred_family);
 			if (dst.family == AF_UNSPEC)
-				invarg("\"dst\" address family is AF_UNSPEC", *argv);
+				invarg("value after \"dst\" has an unrecognized address family", *argv);
 			if (family)
 				*family = dst.family;
 
@@ -1038,7 +1038,7 @@ int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 
 			ret = xfrm_xfrmproto_getbyname(*argv);
 			if (ret < 0)
-				invarg("\"XFRM-PROTO\" is invalid", *argv);
+				invarg("XFRM-PROTO value is invalid", *argv);
 
 			id->proto = (__u8)ret;
 
@@ -1049,7 +1049,7 @@ int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 
 			NEXT_ARG();
 			if (get_u32(&spi, *argv, 0))
-				invarg("\"SPI\" is invalid", *argv);
+				invarg("SPI value is invalid", *argv);
 
 			spi = htonl(spi);
 			id->spi = spi;
@@ -1067,7 +1067,7 @@ int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
 	}
 
 	if (src.family && dst.family && (src.family != dst.family))
-		invarg("the same address family is required between \"src\" and \"dst\"", *argv);
+		invarg("the same address family is required between values after \"src\" and \"dst\"", *argv);
 
 	if (id->spi && id->proto) {
 		if (xfrm_xfrmproto_is_ro(id->proto)) {
@@ -1108,7 +1108,7 @@ int xfrm_mode_parse(__u8 *mode, int *argcp, char ***argvp)
 	else if (matches(*argv, "beet") == 0)
 		*mode = XFRM_MODE_BEET;
 	else
-		invarg("\"MODE\" is invalid", *argv);
+		invarg("MODE value is invalid", *argv);
 
 	*argcp = argc;
 	*argvp = argv;
@@ -1126,7 +1126,7 @@ int xfrm_encap_type_parse(__u16 *type, int *argcp, char ***argvp)
 	else if (strcmp(*argv, "espinudp") == 0)
 		*type = 2;
 	else
-		invarg("\"ENCAP-TYPE\" is invalid", *argv);
+		invarg("ENCAP-TYPE value is invalid", *argv);
 
 	*argcp = argc;
 	*argvp = argv;
@@ -1141,7 +1141,7 @@ int xfrm_reqid_parse(__u32 *reqid, int *argcp, char ***argvp)
 	char **argv = *argvp;
 
 	if (get_u32(reqid, *argv, 0))
-		invarg("\"REQID\" is invalid", *argv);
+		invarg("REQID value is invalid", *argv);
 
 	*argcp = argc;
 	*argvp = argv;
@@ -1175,7 +1175,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 					upspec = pp->p_proto;
 				else {
 					if (get_u8(&upspec, *argv, 0))
-						invarg("\"PROTO\" is invalid", *argv);
+						invarg("PROTO value is invalid", *argv);
 				}
 			}
 			sel->proto = upspec;
@@ -1188,7 +1188,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 			NEXT_ARG();
 
 			if (get_u16(&sel->sport, *argv, 0))
-				invarg("\"PORT\" is invalid", *argv);
+				invarg("value after \"sport\" is invalid", *argv);
 			sel->sport = htons(sel->sport);
 			if (sel->sport)
 				sel->sport_mask = ~((__u16)0);
@@ -1201,7 +1201,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 			NEXT_ARG();
 
 			if (get_u16(&sel->dport, *argv, 0))
-				invarg("\"PORT\" is invalid", *argv);
+				invarg("value after \"dport\" is invalid", *argv);
 			sel->dport = htons(sel->dport);
 			if (sel->dport)
 				sel->dport_mask = ~((__u16)0);
@@ -1215,7 +1215,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 
 			if (get_u16(&sel->sport, *argv, 0) ||
 			    (sel->sport & ~((__u16)0xff)))
-				invarg("\"type\" value is invalid", *argv);
+				invarg("value after \"type\" is invalid", *argv);
 			sel->sport = htons(sel->sport);
 			sel->sport_mask = ~((__u16)0);
 
@@ -1229,7 +1229,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 
 			if (get_u16(&sel->dport, *argv, 0) ||
 			    (sel->dport & ~((__u16)0xff)))
-				invarg("\"code\" value is invalid", *argv);
+				invarg("value after \"code\" is invalid", *argv);
 			sel->dport = htons(sel->dport);
 			sel->dport_mask = ~((__u16)0);
 
@@ -1246,7 +1246,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 				uval = htonl(get_addr32(*argv));
 			else {
 				if (get_unsigned(&uval, *argv, 0)<0) {
-					fprintf(stderr, "invalid value for \"key\"; it should be an unsigned integer\n");
+					fprintf(stderr, "value after \"key\" is invalid\n");
 					exit(-1);
 				}
 			}
@@ -1277,7 +1277,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 		case IPPROTO_DCCP:
 			break;
 		default:
-			fprintf(stderr, "\"sport\" and \"dport\" are invalid with proto=%s\n", strxf_proto(sel->proto));
+			fprintf(stderr, "\"sport\" and \"dport\" are invalid with PROTO value \"%s\"\n", strxf_proto(sel->proto));
 			exit(1);
 		}
 	}
@@ -1288,7 +1288,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 		case IPPROTO_MH:
 			break;
 		default:
-			fprintf(stderr, "\"type\" and \"code\" are invalid with proto=%s\n", strxf_proto(sel->proto));
+			fprintf(stderr, "\"type\" and \"code\" are invalid with PROTO value \"%s\"\n", strxf_proto(sel->proto));
 			exit(1);
 		}
 	}
@@ -1297,7 +1297,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 		case IPPROTO_GRE:
 			break;
 		default:
-			fprintf(stderr, "\"key\" is invalid with proto=%s\n", strxf_proto(sel->proto));
+			fprintf(stderr, "\"key\" is invalid with PROTO value \"%s\"\n", strxf_proto(sel->proto));
 			exit(1);
 		}
 	}
@@ -1325,7 +1325,7 @@ int xfrm_selector_parse(struct xfrm_selector *sel, int *argcp, char ***argvp)
 
 			get_prefix(&src, *argv, preferred_family);
 			if (src.family == AF_UNSPEC)
-				invarg("\"src\" address family is AF_UNSPEC", *argv);
+				invarg("value after \"src\" has an unrecognized address family", *argv);
 			sel->family = src.family;
 
 			memcpy(&sel->saddr, &src.data, sizeof(sel->saddr));
@@ -1338,7 +1338,7 @@ int xfrm_selector_parse(struct xfrm_selector *sel, int *argcp, char ***argvp)
 
 			get_prefix(&dst, *argv, preferred_family);
 			if (dst.family == AF_UNSPEC)
-				invarg("\"dst\" address family is AF_UNSPEC", *argv);
+				invarg("value after \"dst\" has an unrecognized address family", *argv);
 			sel->family = dst.family;
 
 			memcpy(&sel->daddr, &dst.data, sizeof(sel->daddr));
@@ -1356,7 +1356,7 @@ int xfrm_selector_parse(struct xfrm_selector *sel, int *argcp, char ***argvp)
 			else {
 				ifindex = ll_name_to_index(*argv);
 				if (ifindex <= 0)
-					invarg("\"DEV\" is invalid", *argv);
+					invarg("DEV value is invalid", *argv);
 			}
 			sel->ifindex = ifindex;
 
@@ -1379,7 +1379,7 @@ int xfrm_selector_parse(struct xfrm_selector *sel, int *argcp, char ***argvp)
 	}
 
 	if (src.family && dst.family && (src.family != dst.family))
-		invarg("the same address family is required between \"src\" and \"dst\"", *argv);
+		invarg("the same address family is required between values after \"src\" and \"dst\"", *argv);
 
 	if (argc == *argcp)
 		missarg("SELECTOR");
@@ -1401,44 +1401,44 @@ int xfrm_lifetime_cfg_parse(struct xfrm_lifetime_cfg *lft,
 		NEXT_ARG();
 		ret = get_u64(&lft->soft_add_expires_seconds, *argv, 0);
 		if (ret)
-			invarg("\"time-soft\" value is invalid", *argv);
+			invarg("value after \"time-soft\" is invalid", *argv);
 	} else if (strcmp(*argv, "time-hard") == 0) {
 		NEXT_ARG();
 		ret = get_u64(&lft->hard_add_expires_seconds, *argv, 0);
 		if (ret)
-			invarg("\"time-hard\" value is invalid", *argv);
+			invarg("value after \"time-hard\" is invalid", *argv);
 	} else if (strcmp(*argv, "time-use-soft") == 0) {
 		NEXT_ARG();
 		ret = get_u64(&lft->soft_use_expires_seconds, *argv, 0);
 		if (ret)
-			invarg("\"time-use-soft\" value is invalid", *argv);
+			invarg("value after \"time-use-soft\" is invalid", *argv);
 	} else if (strcmp(*argv, "time-use-hard") == 0) {
 		NEXT_ARG();
 		ret = get_u64(&lft->hard_use_expires_seconds, *argv, 0);
 		if (ret)
-			invarg("\"time-use-hard\" value is invalid", *argv);
+			invarg("value after \"time-use-hard\" is invalid", *argv);
 	} else if (strcmp(*argv, "byte-soft") == 0) {
 		NEXT_ARG();
 		ret = get_u64(&lft->soft_byte_limit, *argv, 0);
 		if (ret)
-			invarg("\"byte-soft\" value is invalid", *argv);
+			invarg("value after \"byte-soft\" is invalid", *argv);
 	} else if (strcmp(*argv, "byte-hard") == 0) {
 		NEXT_ARG();
 		ret = get_u64(&lft->hard_byte_limit, *argv, 0);
 		if (ret)
-			invarg("\"byte-hard\" value is invalid", *argv);
+			invarg("value after \"byte-hard\" is invalid", *argv);
 	} else if (strcmp(*argv, "packet-soft") == 0) {
 		NEXT_ARG();
 		ret = get_u64(&lft->soft_packet_limit, *argv, 0);
 		if (ret)
-			invarg("\"packet-soft\" value is invalid", *argv);
+			invarg("value after \"packet-soft\" is invalid", *argv);
 	} else if (strcmp(*argv, "packet-hard") == 0) {
 		NEXT_ARG();
 		ret = get_u64(&lft->hard_packet_limit, *argv, 0);
 		if (ret)
-			invarg("\"packet-hard\" value is invalid", *argv);
+			invarg("value after \"packet-hard\" is invalid", *argv);
 	} else
-		invarg("\"LIMIT\" is invalid", *argv);
+		invarg("LIMIT value is invalid", *argv);
 
 	*argcp = argc;
 	*argvp = argv;
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 27c9a65..9bc584e 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -114,7 +114,7 @@ static int xfrm_policy_dir_parse(__u8 *dir, int *argcp, char ***argvp)
 	else if (strcmp(*argv, "fwd") == 0)
 		*dir = XFRM_POLICY_FWD;
 	else
-		invarg("\"DIR\" is invalid", *argv);
+		invarg("DIR value is invalid", *argv);
 
 	*argcp = argc;
 	*argvp = argv;
@@ -132,7 +132,7 @@ static int xfrm_policy_ptype_parse(__u8 *ptype, int *argcp, char ***argvp)
 	else if (strcmp(*argv, "sub") == 0)
 		*ptype = XFRM_POLICY_TYPE_SUB;
 	else
-		invarg("\"PTYPE\" is invalid", *argv);
+		invarg("PTYPE value is invalid", *argv);
 
 	*argcp = argc;
 	*argvp = argv;
@@ -150,7 +150,7 @@ static int xfrm_policy_flag_parse(__u8 *flags, int *argcp, char ***argvp)
 		__u8 val = 0;
 
 		if (get_u8(&val, *argv, 16))
-			invarg("\"FLAG\" is invalid", *argv);
+			invarg("FLAG value is invalid", *argv);
 		*flags = val;
 	} else {
 		while (1) {
@@ -197,7 +197,7 @@ static int xfrm_tmpl_parse(struct xfrm_user_tmpl *tmpl,
 			else if (strcmp(*argv, "use") == 0)
 				tmpl->optional = 1;
 			else
-				invarg("\"LEVEL\" is invalid\n", *argv);
+				invarg("LEVEL value is invalid\n", *argv);
 
 		} else {
 			if (idp) {
@@ -300,7 +300,7 @@ static int xfrm_policy_modify(int cmd, unsigned flags, int argc, char **argv)
 		} else if (strcmp(*argv, "index") == 0) {
 			NEXT_ARG();
 			if (get_u32(&req.xpinfo.index, *argv, 0))
-				invarg("\"INDEX\" is invalid", *argv);
+				invarg("INDEX value is invalid", *argv);
 		} else if (strcmp(*argv, "ptype") == 0) {
 			if (ptypep)
 				duparg("ptype", *argv);
@@ -315,11 +315,11 @@ static int xfrm_policy_modify(int cmd, unsigned flags, int argc, char **argv)
 			else if (strcmp(*argv, "block") == 0)
 				req.xpinfo.action = XFRM_POLICY_BLOCK;
 			else
-				invarg("\"action\" value is invalid\n", *argv);
+				invarg("ACTION value is invalid\n", *argv);
 		} else if (strcmp(*argv, "priority") == 0) {
 			NEXT_ARG();
 			if (get_u32(&req.xpinfo.priority, *argv, 0))
-				invarg("\"PRIORITY\" is invalid", *argv);
+				invarg("PRIORITY value is invalid", *argv);
 		} else if (strcmp(*argv, "flag") == 0) {
 			NEXT_ARG();
 			xfrm_policy_flag_parse(&req.xpinfo.flags, &argc,
@@ -359,7 +359,7 @@ static int xfrm_policy_modify(int cmd, unsigned flags, int argc, char **argv)
 	}
 
 	if (!dirp) {
-		fprintf(stderr, "Not enough information: \"DIR\" is required.\n");
+		fprintf(stderr, "Not enough information: DIR is required.\n");
 		exit(1);
 	}
 
@@ -611,7 +611,7 @@ static int xfrm_policy_get_or_delete(int argc, char **argv, int delete,
 
 			NEXT_ARG();
 			if (get_u32(&req.xpid.index, *argv, 0))
-				invarg("\"INDEX\" is invalid", *argv);
+				invarg("INDEX value is invalid", *argv);
 
 		} else if (strcmp(*argv, "ptype") == 0) {
 			if (ptypep)
@@ -636,7 +636,7 @@ static int xfrm_policy_get_or_delete(int argc, char **argv, int delete,
 	}
 
 	if (!dirp) {
-		fprintf(stderr, "Not enough information: \"DIR\" is required.\n");
+		fprintf(stderr, "Not enough information: DIR is required.\n");
 		exit(1);
 	}
 	if (ptypep) {
@@ -644,7 +644,7 @@ static int xfrm_policy_get_or_delete(int argc, char **argv, int delete,
 			  (void *)&upt, sizeof(upt));
 	}
 	if (!selp && !indexp) {
-		fprintf(stderr, "Not enough information: either \"SELECTOR\" or \"INDEX\" is required.\n");
+		fprintf(stderr, "Not enough information: either SELECTOR or INDEX is required.\n");
 		exit(1);
 	}
 	if (selp && indexp)
@@ -786,7 +786,7 @@ static int xfrm_policy_list_or_deleteall(int argc, char **argv, int deleteall)
 		} else if (strcmp(*argv, "index") == 0) {
 			NEXT_ARG();
 			if (get_u32(&filter.xpinfo.index, *argv, 0))
-				invarg("\"INDEX\" is invalid", *argv);
+				invarg("INDEX value is invalid", *argv);
 
 			filter.index_mask = XFRM_FILTER_MASK_FULL;
 
@@ -803,14 +803,14 @@ static int xfrm_policy_list_or_deleteall(int argc, char **argv, int deleteall)
 			else if (strcmp(*argv, "block") == 0)
 				filter.xpinfo.action = XFRM_POLICY_BLOCK;
 			else
-				invarg("\"ACTION\" is invalid\n", *argv);
+				invarg("ACTION value is invalid\n", *argv);
 
 			filter.action_mask = XFRM_FILTER_MASK_FULL;
 
 		} else if (strcmp(*argv, "priority") == 0) {
 			NEXT_ARG();
 			if (get_u32(&filter.xpinfo.priority, *argv, 0))
-				invarg("\"PRIORITY\" is invalid", *argv);
+				invarg("PRIORITY value is invalid", *argv);
 
 			filter.priority_mask = XFRM_FILTER_MASK_FULL;
 
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index ee06f7d..88a1a56 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -175,7 +175,7 @@ static int xfrm_seq_parse(__u32 *seq, int *argcp, char ***argvp)
 	char **argv = *argvp;
 
 	if (get_u32(seq, *argv, 0))
-		invarg("\"SEQ\" is invalid", *argv);
+		invarg("SEQ value is invalid", *argv);
 
 	*seq = htonl(*seq);
 
@@ -195,7 +195,7 @@ static int xfrm_state_flag_parse(__u8 *flags, int *argcp, char ***argvp)
 		__u8 val = 0;
 
 		if (get_u8(&val, *argv, 16))
-			invarg("\"FLAG\" is invalid", *argv);
+			invarg("FLAG value is invalid", *argv);
 		*flags = val;
 	} else {
 		while (1) {
@@ -281,15 +281,15 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 		} else if (strcmp(*argv, "replay-window") == 0) {
 			NEXT_ARG();
 			if (get_u8(&req.xsinfo.replay_window, *argv, 0))
-				invarg("\"replay-window\" value is invalid", *argv);
+				invarg("value after \"replay-window\" is invalid", *argv);
 		} else if (strcmp(*argv, "replay-seq") == 0) {
 			NEXT_ARG();
 			if (get_u32(&replay.seq, *argv, 0))
-				invarg("\"replay-seq\" value is invalid", *argv);
+				invarg("value after \"replay-seq\" is invalid", *argv);
 		} else if (strcmp(*argv, "replay-oseq") == 0) {
 			NEXT_ARG();
 			if (get_u32(&replay.oseq, *argv, 0))
-				invarg("\"replay-oseq\" value is invalid", *argv);
+				invarg("value after \"replay-oseq\" is invalid", *argv);
 		} else if (strcmp(*argv, "flag") == 0) {
 			NEXT_ARG();
 			xfrm_state_flag_parse(&req.xsinfo.flags, &argc, &argv);
@@ -308,11 +308,11 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 			xfrm_encap_type_parse(&encap.encap_type, &argc, &argv);
 			NEXT_ARG();
 			if (get_u16(&encap.encap_sport, *argv, 0))
-				invarg("\"encap\" sport value is invalid", *argv);
+				invarg("SPORT value after \"encap\" is invalid", *argv);
 			encap.encap_sport = htons(encap.encap_sport);
 			NEXT_ARG();
 			if (get_u16(&encap.encap_dport, *argv, 0))
-				invarg("\"encap\" dport value is invalid", *argv);
+				invarg("DPORT value after \"encap\" is invalid", *argv);
 			encap.encap_dport = htons(encap.encap_dport);
 			NEXT_ARG();
 			get_addr(&oa, *argv, AF_UNSPEC);
@@ -331,9 +331,9 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 
 			get_prefix(&coa, *argv, preferred_family);
 			if (coa.family == AF_UNSPEC)
-				invarg("\"coa\" address family is AF_UNSPEC", *argv);
+				invarg("value after \"coa\" has an unrecognized address family", *argv);
 			if (coa.bytelen > sizeof(xcoa))
-				invarg("\"coa\" address length is too large", *argv);
+				invarg("value after \"coa\" is too large", *argv);
 
 			memset(&xcoa, 0, sizeof(xcoa));
 			memcpy(&xcoa, &coa.data, coa.bytelen);
@@ -402,7 +402,7 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 					break;
 				default:
 					/* not reached */
-					invarg("\"ALGO-TYPE\" is invalid\n", *argv);
+					invarg("ALGO-TYPE value is invalid\n", *argv);
 				}
 
 				if (!NEXT_ARG_OK())
@@ -431,7 +431,7 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 						missarg("ALGO-ICV-LEN");
 					NEXT_ARG();
 					if (get_u32(&icvlen, *argv, 0))
-						invarg("\"aead\" ICV length is invalid",
+						invarg("ALGO-ICV-LEN value is invalid",
 						       *argv);
 					alg.u.aead.alg_icv_len = icvlen;
 
@@ -443,7 +443,7 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 						missarg("ALGO-TRUNC-LEN");
 					NEXT_ARG();
 					if (get_u32(&trunclen, *argv, 0))
-						invarg("\"auth\" trunc length is invalid",
+						invarg("ALGO-TRUNC-LEN value is invalid",
 						       *argv);
 					alg.u.auth.alg_trunc_len = trunclen;
 
@@ -481,7 +481,7 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 			  (void *)&replay, sizeof(replay));
 
 	if (!idp) {
-		fprintf(stderr, "Not enough information: \"ID\" is required\n");
+		fprintf(stderr, "Not enough information: ID is required\n");
 		exit(1);
 	}
 
@@ -660,7 +660,7 @@ static int xfrm_state_allocspi(int argc, char **argv)
 			NEXT_ARG();
 
 			if (get_u32(&req.xspi.min, *argv, 0))
-				invarg("\"min\" value is invalid", *argv);
+				invarg("value after \"min\" is invalid", *argv);
 		} else if (strcmp(*argv, "max") == 0) {
 			if (maxp)
 				duparg("max", *argv);
@@ -669,7 +669,7 @@ static int xfrm_state_allocspi(int argc, char **argv)
 			NEXT_ARG();
 
 			if (get_u32(&req.xspi.max, *argv, 0))
-				invarg("\"max\" value is invalid", *argv);
+				invarg("value after \"max\" is invalid", *argv);
 		} else {
 			/* try to assume ID */
 			if (idp)
@@ -680,7 +680,7 @@ static int xfrm_state_allocspi(int argc, char **argv)
 			xfrm_id_parse(&req.xspi.info.saddr, &req.xspi.info.id,
 				      &req.xspi.info.family, 0, &argc, &argv);
 			if (req.xspi.info.id.spi) {
-				fprintf(stderr, "\"SPI\" must be zero\n");
+				fprintf(stderr, "\"spi\" is invalid\n");
 				exit(1);
 			}
 			if (preferred_family == AF_UNSPEC)
@@ -690,7 +690,7 @@ static int xfrm_state_allocspi(int argc, char **argv)
 	}
 
 	if (!idp) {
-		fprintf(stderr, "Not enough information: \"ID\" is required\n");
+		fprintf(stderr, "Not enough information: ID is required\n");
 		exit(1);
 	}
 
@@ -700,7 +700,7 @@ static int xfrm_state_allocspi(int argc, char **argv)
 			exit(1);
 		}
 		if (req.xspi.min > req.xspi.max) {
-			fprintf(stderr, "\"min\" value is larger than \"max\" value\n");
+			fprintf(stderr, "value after \"min\" is larger than value after \"max\"\n");
 			exit(1);
 		}
 	} else {
@@ -1215,7 +1215,7 @@ static int xfrm_state_flush(int argc, char **argv)
 
 			ret = xfrm_xfrmproto_getbyname(*argv);
 			if (ret < 0)
-				invarg("\"XFRM-PROTO\" is invalid", *argv);
+				invarg("XFRM-PROTO value is invalid", *argv);
 
 			req.xsf.proto = (__u8)ret;
 		} else
@@ -1228,7 +1228,7 @@ static int xfrm_state_flush(int argc, char **argv)
 		exit(1);
 
 	if (show_stats > 1)
-		fprintf(stderr, "Flush state proto=%s\n",
+		fprintf(stderr, "Flush state with XFRM-PROTO value \"%s\"\n",
 			strxf_xfrmproto(req.xsf.proto));
 
 	if (rtnl_talk(&rth, &req.n, 0, 0, NULL) < 0)
-- 
1.7.1

^ permalink raw reply related

* [PATCH iproute2 6/7] ip/xfrm: Improve usage text and documentation
From: David Ward @ 2013-03-25 14:23 UTC (permalink / raw)
  To: netdev; +Cc: David Ward
In-Reply-To: <1364221399-1024-1-git-send-email-david.ward@ll.mit.edu>

Change ALGO-KEY to ALGO-KEYMAT to make it more obvious that the
keying material might need to contain more than just the key (such
as a salt or nonce value).

List the algorithm names that currently exist in the kernel.

Indicate that for IPComp, the Compression Parameter Index (CPI) is
used as the SPI.

Group the list of mode values by transform protocol.

Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 ip/xfrm_policy.c   |    2 +-
 ip/xfrm_state.c    |   18 ++++----
 man/man8/ip-xfrm.8 |  112 +++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 85 insertions(+), 47 deletions(-)

diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index bf263e0..27c9a65 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -96,7 +96,7 @@ static void usage(void)
 	fprintf(stderr, "%s | ", strxf_xfrmproto(IPPROTO_COMP));
 	fprintf(stderr, "%s | ", strxf_xfrmproto(IPPROTO_ROUTING));
 	fprintf(stderr, "%s\n", strxf_xfrmproto(IPPROTO_DSTOPTS));
- 	fprintf(stderr, "MODE := transport | tunnel | ro | in_trigger | beet\n");
+ 	fprintf(stderr, "MODE := transport | tunnel | beet | ro | in_trigger\n");
 	fprintf(stderr, "LEVEL := required | use\n");
 
 	exit(-1);
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index 9b374ee..ee06f7d 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -79,14 +79,14 @@ static void usage(void)
 	fprintf(stderr, "ALGO := { ");
 	fprintf(stderr, "%s | ", strxf_algotype(XFRMA_ALG_CRYPT));
 	fprintf(stderr, "%s", strxf_algotype(XFRMA_ALG_AUTH));
-	fprintf(stderr, " } ALGO-NAME ALGO-KEY |\n");
+	fprintf(stderr, " } ALGO-NAME ALGO-KEYMAT |\n");
 	fprintf(stderr, "        %s", strxf_algotype(XFRMA_ALG_AUTH_TRUNC));
-	fprintf(stderr, " ALGO-NAME ALGO-KEY ALGO-TRUNC-LEN |\n");
+	fprintf(stderr, " ALGO-NAME ALGO-KEYMAT ALGO-TRUNC-LEN |\n");
 	fprintf(stderr, "        %s", strxf_algotype(XFRMA_ALG_AEAD));
-	fprintf(stderr, " ALGO-NAME ALGO-KEY ALGO-ICV-LEN |\n");
+	fprintf(stderr, " ALGO-NAME ALGO-KEYMAT ALGO-ICV-LEN |\n");
 	fprintf(stderr, "        %s", strxf_algotype(XFRMA_ALG_COMP));
 	fprintf(stderr, " ALGO-NAME\n");
- 	fprintf(stderr, "MODE := transport | tunnel | ro | in_trigger | beet\n");
+ 	fprintf(stderr, "MODE := transport | tunnel | beet | ro | in_trigger\n");
 	fprintf(stderr, "FLAG-LIST := [ FLAG-LIST ] FLAG\n");
 	fprintf(stderr, "FLAG := noecn | decap-dscp | nopmtudisc | wildrecv | icmp | af-unspec | align4\n");
 	fprintf(stderr, "SELECTOR := [ src ADDR[/PLEN] ] [ dst ADDR[/PLEN] ] [ dev DEV ] [ UPSPEC ]\n");
@@ -119,7 +119,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
 
 #if 0
 	/* XXX: verifying both name and key is required! */
-	fprintf(stderr, "warning: ALGO-NAME/ALGO-KEY will send to kernel promiscuously! (verifying them isn't implemented yet)\n");
+	fprintf(stderr, "warning: ALGO-NAME/ALGO-KEYMAT values will be sent to the kernel promiscuously! (verifying them isn't implemented yet)\n");
 #endif
 
 	strncpy(alg->alg_name, name, sizeof(alg->alg_name));
@@ -139,7 +139,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
 		/* calculate length of the converted values(real key) */
 		len = (plen + 1) / 2;
 		if (len > max)
-			invarg("\"ALGO-KEY\" makes buffer overflow\n", key);
+			invarg("ALGO-KEYMAT value makes buffer overflow\n", key);
 
 		for (i = - (plen % 2), j = 0; j < len; i += 2, j++) {
 			char vbuf[3];
@@ -150,7 +150,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
 			vbuf[2] = '\0';
 
 			if (get_u8(&val, vbuf, 16))
-				invarg("\"ALGO-KEY\" is invalid", key);
+				invarg("ALGO-KEYMAT value is invalid", key);
 
 			buf[j] = val;
 		}
@@ -158,7 +158,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
 		len = slen;
 		if (len > 0) {
 			if (len > max)
-				invarg("\"ALGO-KEY\" makes buffer overflow\n", key);
+				invarg("ALGO-KEYMAT value makes buffer overflow\n", key);
 
 			strncpy(buf, key, len);
 		}
@@ -416,7 +416,7 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
 				case XFRMA_ALG_AUTH:
 				case XFRMA_ALG_AUTH_TRUNC:
 					if (!NEXT_ARG_OK())
-						missarg("ALGO-KEY");
+						missarg("ALGO-KEYMAT");
 					NEXT_ARG();
 					key = *argv;
 					break;
diff --git a/man/man8/ip-xfrm.8 b/man/man8/ip-xfrm.8
index 6017bc2..1d33eed 100644
--- a/man/man8/ip-xfrm.8
+++ b/man/man8/ip-xfrm.8
@@ -118,20 +118,20 @@ ip-xfrm \- transform configuration
 .ti -8
 .IR ALGO " :="
 .RB "{ " enc " | " auth " } " 
-.IR ALGO-NAME " " ALGO-KEY " |"
+.IR ALGO-NAME " " ALGO-KEYMAT " |"
 .br
 .B auth-trunc
-.IR ALGO-NAME " " ALGO-KEY " " ALGO-TRUNC-LEN " |"
+.IR ALGO-NAME " " ALGO-KEYMAT " " ALGO-TRUNC-LEN " |"
 .br
 .B aead
-.IR ALGO-NAME " " ALGO-KEY " " ALGO-ICV-LEN " |"
+.IR ALGO-NAME " " ALGO-KEYMAT " " ALGO-ICV-LEN " |"
 .br
 .B comp
 .IR ALGO-NAME
 
 .ti -8
 .IR MODE " := "
-.BR transport " | " tunnel " | " ro " | " in_trigger " | " beet
+.BR transport " | " tunnel " | " beet " | " ro " | " in_trigger
 
 .ti -8
 .IR FLAG-LIST " := [ " FLAG-LIST " ] " FLAG
@@ -345,7 +345,7 @@ ip-xfrm \- transform configuration
 
 .ti -8
 .IR MODE " := "
-.BR transport " | " tunnel " | " ro " | " in_trigger " | " beet
+.BR transport " | " tunnel " | " beet " | " ro " | " in_trigger
 
 .ti -8
 .IR LEVEL " :="
@@ -393,6 +393,8 @@ is specified by a source address, destination address,
 .RI "transform protocol " XFRM-PROTO ","
 and/or Security Parameter Index
 .IR SPI "."
+(For IP Payload Compression, the Compression Parameter Index or CPI is used for
+.IR SPI ".)"
 
 .TP
 .I XFRM-PROTO
@@ -405,37 +407,68 @@ specifies a transform protocol:
 
 .TP
 .I ALGO-LIST
-specifies one or more algorithms
-.IR ALGO
-to use. Algorithm types include
+contains one or more algorithms to use. Each algorithm
+.I ALGO
+is specified by:
+.RS
+.IP \[bu]
+the algorithm type:
 .RB "encryption (" enc "),"
-.RB "authentication (" auth "),"
-.RB "authentication with a specified truncation length (" auth-trunc "),"
-.RB "authenticated encryption with associated data (" aead "), and"
-.RB "compression (" comp ")."
-For each algorithm used, the algorithm type, the algorithm name
-.IR ALGO-NAME ","
-and the key
-.I ALGO-KEY
-must be specified. For
-.BR aead ","
+.RB "authentication (" auth " or " auth-trunc "),"
+.RB "authenticated encryption with associated data (" aead "), or"
+.RB "compression (" comp ")"
+.IP \[bu]
+the algorithm name
+.IR ALGO-NAME
+(see below)
+.IP \[bu]
+.RB "(for all except " comp ")"
+the keying material
+.IR ALGO-KEYMAT ","
+which may include both a key and a salt or nonce value; refer to the
+corresponding RFC
+.IP \[bu]
+.RB "(for " auth-trunc " only)"
+the truncation length
+.I ALGO-TRUNC-LEN
+in bits
+.IP \[bu]
+.RB "(for " aead " only)"
 the Integrity Check Value length
 .I ALGO-ICV-LEN
-must additionally be specified.
-For
-.BR auth-trunc ","
-the signature truncation length
-.I ALGO-TRUNC-LEN
-must additionally be specified.
+in bits
+.RE
+
+.nh
+.RS
+Encryption algorithms include
+.BR ecb(cipher_null) ", " cbc(des) ", " cbc(des3_ede) ", " cbc(cast5) ","
+.BR cbc(blowfish) ", " cbc(aes) ", " cbc(serpent) ", " cbc(camellia) ","
+.BR cbc(twofish) ", and " rfc3686(ctr(aes)) "."
+
+Authentication algorithms include
+.BR digest_null ", " hmac(md5) ", " hmac(sha1) ", " hmac(sha256) ","
+.BR hmac(sha384) ", " hmac(sha512) ", " hmac(rmd610) ", and " xcbc(aes) "."
+
+Authenticated encryption with associated data (AEAD) algorithms include
+.BR rfc4106(gcm(aes)) ", " rfc4309(ccm(aes)) ", and " rfc4543(gcm(aes)) "."
+
+Compression algorithms include
+.BR deflate ", " lzs ", and " lzjh "."
+.RE
+.hy
 
 .TP
 .I MODE
-specifies a mode of operation:
-.RB "IPsec transport mode (" transport "), "
-.RB "IPsec tunnel mode (" tunnel "), "
-.RB "Mobile IPv6 route optimization mode (" ro "), "
-.RB "Mobile IPv6 inbound trigger mode (" in_trigger "), or "
-.RB "IPsec ESP Bound End-to-End Tunnel Mode (" beet ")."
+specifies a mode of operation for the transform protocol. IPsec and IP Payload
+Compression modes are
+.BR transport ", " tunnel ","
+and (for IPsec ESP only) Bound End-to-End Tunnel
+.RB "(" beet ")."
+Mobile IPv6 modes are route optimization
+.RB "(" ro ")"
+and inbound trigger
+.RB "(" in_trigger ")."
 
 .TP
 .I FLAG-LIST
@@ -553,6 +586,8 @@ is specified by a source address, destination address,
 .RI "transform protocol " XFRM-PROTO ","
 and/or Security Parameter Index
 .IR SPI "."
+(For IP Payload Compression, the Compression Parameter Index or CPI is used for
+.IR SPI ".)"
 
 .TP
 .I XFRM-PROTO
@@ -565,12 +600,15 @@ specifies a transform protocol:
 
 .TP
 .I MODE
-specifies a mode of operation:
-.RB "IPsec transport mode (" transport "), "
-.RB "IPsec tunnel mode (" tunnel "), "
-.RB "Mobile IPv6 route optimization mode (" ro "), "
-.RB "Mobile IPv6 inbound trigger mode (" in_trigger "), or "
-.RB "IPsec ESP Bound End-to-End Tunnel Mode (" beet ")."
+specifies a mode of operation for the transform protocol. IPsec and IP Payload
+Compression modes are
+.BR transport ", " tunnel ","
+and (for IPsec ESP only) Bound End-to-End Tunnel
+.RB "(" beet ")."
+Mobile IPv6 modes are route optimization
+.RB "(" ro ")"
+and inbound trigger
+.RB "(" in_trigger ")."
 
 .TP
 .I LEVEL
@@ -581,4 +619,4 @@ can be
 The xfrm objects to monitor can be optionally specified.
 
 .SH AUTHOR
-Manpage by David Ward
+Manpage revised by David Ward <david.ward@ll.mit.edu>
-- 
1.7.1

^ permalink raw reply related

* Re: [Xen-devel] [PATCH 2/6] xen-netfront: reduce gso_max_size to account for ethernet header
From: David Vrabel @ 2013-03-25 14:23 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, netdev, annie.li, david.vrabel, ian.campbell,
	konrad.wilk
In-Reply-To: <1364209702-12437-3-git-send-email-wei.liu2@citrix.com>

On 25/03/13 11:08, Wei Liu wrote:
> The maximum packet including ethernet header that can be handled by netfront /
> netback wire format is 65535. Reduce gso_max_size accordingly.
> 
> Drop skb and print warning when skb->len > 65535. This can 1) save the effort
> to send malformed packet to netback, 2) help spotting misconfiguration of
> netfront in the future.
[...]
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -547,6 +547,18 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	unsigned int len = skb_headlen(skb);
>  	unsigned long flags;
>  
> +	/* Wire format of xen_netif_tx_request only supports skb->len
> +	 * < 64K, because size field in xen_netif_tx_request is
> +	 * uint16_t. If skb->len is too big, drop it and alert user about
> +	 * misconfiguration.
> +	 */
> +	if (unlikely(skb->len >= (uint16_t)(~0))) {

Suggest a #define for this maximum added to include/xen/interface/io/netif.h

David

^ permalink raw reply

* Re: [Xen-devel] [PATCH 3/6] xen-netfront: frags -> slots in xennet_get_responses
From: David Vrabel @ 2013-03-25 14:25 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, netdev, annie.li, david.vrabel, ian.campbell,
	konrad.wilk
In-Reply-To: <1364209702-12437-4-git-send-email-wei.liu2@citrix.com>

On 25/03/13 11:08, Wei Liu wrote:
> This function is in fact counting the ring slots required for responses.
> Separate the concepts of ring slots and skb frags make the code clearer, as
> now netfront and netback can have different MAX_SKB_FRAGS, slot and frag are
> not mapped 1:1 any more.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Makes sense.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

^ permalink raw reply

* Re: [Xen-devel] [PATCH 2/6] xen-netfront: reduce gso_max_size to account for ethernet header
From: Jan Beulich @ 2013-03-25 14:39 UTC (permalink / raw)
  To: david.vrabel, Wei Liu
  Cc: ian.campbell, xen-devel, annie.li, konrad.wilk, netdev
In-Reply-To: <51505DF9.4080406@citrix.com>

>>> On 25.03.13 at 15:23, David Vrabel <david.vrabel@citrix.com> wrote:
> On 25/03/13 11:08, Wei Liu wrote:
>> The maximum packet including ethernet header that can be handled by netfront 
> /
>> netback wire format is 65535. Reduce gso_max_size accordingly.
>> 
>> Drop skb and print warning when skb->len > 65535. This can 1) save the effort
>> to send malformed packet to netback, 2) help spotting misconfiguration of
>> netfront in the future.
> [...]
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
>> @@ -547,6 +547,18 @@ static int xennet_start_xmit(struct sk_buff *skb, struct 
> net_device *dev)
>>  	unsigned int len = skb_headlen(skb);
>>  	unsigned long flags;
>>  
>> +	/* Wire format of xen_netif_tx_request only supports skb->len
>> +	 * < 64K, because size field in xen_netif_tx_request is
>> +	 * uint16_t. If skb->len is too big, drop it and alert user about
>> +	 * misconfiguration.
>> +	 */
>> +	if (unlikely(skb->len >= (uint16_t)(~0))) {
> 
> Suggest a #define for this maximum added to include/xen/interface/io/netif.h

I don't see a point in doing so. If you want the connection to be
explicit, just use typeof() here instead of uint16_t.

Jan

^ permalink raw reply

* Re: [PATCH iproute2 1/7] ip/xfrm: Extend SPI validity checking
From: Stephen Hemminger @ 2013-03-25 15:03 UTC (permalink / raw)
  To: David Ward; +Cc: netdev
In-Reply-To: <1364221399-1024-1-git-send-email-david.ward@ll.mit.edu>

On Mon, 25 Mar 2013 10:23:13 -0400
David Ward <david.ward@ll.mit.edu> wrote:

> A Security Policy Index (SPI) is not used with Mobile IPv6. IPComp
> uses a smaller 16-bit Compression Parameter Index (CPI) which is
> passed as the SPI value. Perform checks whenever specifying an ID.
> 
> Signed-off-by: David Ward <david.ward@ll.mit.edu>

These all look fine to me, I will apply them in a couple of days
unless there is anybody who sees a compatibility problem with current usage.

^ permalink raw reply

* [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli

Hello David,

Please find here 3 minor bugfixes related to my recent device tree bindings
additions to DSA. Thanks!

Florian Fainelli (3):
  dsa: fix device tree binding documentation typo on #address-cells
  dsa: factor freeing of dsa_platform_data
  dsa: fix freeing of sparse port allocation

 Documentation/devicetree/bindings/net/dsa/dsa.txt |    2 +-
 net/dsa/dsa.c                                     |   40 ++++++++++-----------
 2 files changed, 21 insertions(+), 21 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH 1/3] dsa: fix device tree binding documentation typo on #address-cells
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1364223820-26051-1-git-send-email-florian@openwrt.org>

The device tree binding documentation for dsa explicitely states that a
DSA node should have its #address-cells property set to 2, yet the
example still used 1, fix that typo.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 Documentation/devicetree/bindings/net/dsa/dsa.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
index db92f55..49f4f7a 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
@@ -43,7 +43,7 @@ Example:
 
 	dsa@0 {
 		compatible = "marvell,dsa";
-		#address-cells = <1>;
+		#address-cells = <2>;
 		#size-cells = <0>;
 
 		interrupts = <10>;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 3/3] dsa: fix freeing of sparse port allocation
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1364223820-26051-1-git-send-email-florian@openwrt.org>

If we have defined a sparse port allocation which is non-contiguous and
contains gaps, the code freeing port_names will just stop when it
encouters a first NULL port_names, which is not right, we should iterate
over all possible number of ports (DSA_MAX_PORTS) until we are done.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 net/dsa/dsa.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index aa2ff58..0eb5d5e 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -350,9 +350,11 @@ static void dsa_of_free_platform_data(struct dsa_platform_data *pd)
 
 	for (i = 0; i < pd->nr_chips; i++) {
 		port_index = 0;
-		while (pd->chip[i].port_names &&
-			pd->chip[i].port_names[++port_index])
-			kfree(pd->chip[i].port_names[port_index]);
+		while (port_index < DSA_MAX_PORTS) {
+			if (pd->chip[i].port_names[port_index])
+				kfree(pd->chip[i].port_names[port_index]);
+			port_index++;
+		}
 		kfree(pd->chip[i].rtable);
 	}
 	kfree(pd->chip);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/3] dsa: factor freeing of dsa_platform_data
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1364223820-26051-1-git-send-email-florian@openwrt.org>

This patch factors the freeing of the struct dsa_platform_data
manipulated by the driver identically in two places to a single
function.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 net/dsa/dsa.c |   38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 908bc11..aa2ff58 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -343,6 +343,21 @@ out:
 	return ret;
 }
 
+static void dsa_of_free_platform_data(struct dsa_platform_data *pd)
+{
+	int i;
+	int port_index;
+
+	for (i = 0; i < pd->nr_chips; i++) {
+		port_index = 0;
+		while (pd->chip[i].port_names &&
+			pd->chip[i].port_names[++port_index])
+			kfree(pd->chip[i].port_names[port_index]);
+		kfree(pd->chip[i].rtable);
+	}
+	kfree(pd->chip);
+}
+
 static int dsa_of_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -354,7 +369,7 @@ static int dsa_of_probe(struct platform_device *pdev)
 	const char *port_name;
 	int chip_index, port_index;
 	const unsigned int *sw_addr, *port_reg;
-	int ret, i;
+	int ret;
 
 	mdio = of_parse_phandle(np, "dsa,mii-bus", 0);
 	if (!mdio)
@@ -439,14 +454,7 @@ static int dsa_of_probe(struct platform_device *pdev)
 	return 0;
 
 out_free_chip:
-	for (i = 0; i < pd->nr_chips; i++) {
-		port_index = 0;
-		while (pd->chip[i].port_names &&
-			pd->chip[i].port_names[++port_index])
-			kfree(pd->chip[i].port_names[port_index]);
-		kfree(pd->chip[i].rtable);
-	}
-	kfree(pd->chip);
+	dsa_of_free_platform_data(pd);
 out_free:
 	kfree(pd);
 	pdev->dev.platform_data = NULL;
@@ -456,21 +464,11 @@ out_free:
 static void dsa_of_remove(struct platform_device *pdev)
 {
 	struct dsa_platform_data *pd = pdev->dev.platform_data;
-	int i;
-	int port_index;
 
 	if (!pdev->dev.of_node)
 		return;
 
-	for (i = 0; i < pd->nr_chips; i++) {
-		port_index = 0;
-		while (pd->chip[i].port_names &&
-			pd->chip[i].port_names[++port_index])
-			kfree(pd->chip[i].port_names[port_index]);
-		kfree(pd->chip[i].rtable);
-	}
-
-	kfree(pd->chip);
+	dsa_of_free_platform_data(pd);
 	kfree(pd);
 }
 #else
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 5/6] xen-netback: coalesce slots before copying
From: David Vrabel @ 2013-03-25 15:13 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, Ian Campbell,
	annie.li@oracle.com, konrad.wilk@oracle.com
In-Reply-To: <1364209702-12437-6-git-send-email-wei.liu2@citrix.com>

On 25/03/13 11:08, Wei Liu wrote:
> This patch tries to coalesce tx requests when constructing grant copy
> structures. It enables netback to deal with situation when frontend's
> MAX_SKB_FRAGS is larger than backend's MAX_SKB_FRAGS.
> 
> It defines max_skb_slots, which is a estimation of the maximum number of slots
> a guest can send, anything bigger than that is considered malicious. Now it is
> set to 20, which should be enough to accommodate Linux (16 to 19).

This maximum needs to be defined as part of the protocol and added to
the interface header.

> Also change variable name from "frags" to "slots" in netbk_count_requests.

I think this renaming should have been done as a separate patch.

> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -47,11 +47,26 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/page.h>
>  
> +/*
> + * This is an estimation of the maximum possible frags a SKB might
> + * have, anything larger than this is considered malicious. Typically
> + * Linux has 16 to 19.
> + */

Do you mean "max possible /slots/" a packet might have?

> @@ -968,48 +987,114 @@ static struct gnttab_copy *xen_netbk_get_requests(struct xen_netbk *netbk,
[...]
> +				/* Poison these fields, corresponding
> +				 * fields for head tx req will be set
> +				 * to correct values after the loop.
> +				 */
> +				netbk->mmap_pages[pending_idx] = (void *)(~0UL);
> +				pending_tx_info[pending_idx].head =
> +					INVALID_PENDING_RING_IDX;

Do you need to poison both values?

> +
> +				if (unlikely(!first)) {

This isn't unlikely is it?

> +					first = &pending_tx_info[pending_idx];
> +					start_idx = index;
> +					head_idx = pending_idx;
> +				}

Instead of setting first here why not move the code below here?

> +		first->req.offset = 0;
> +		first->req.size = dst_offset;
> +		first->head = start_idx;
> +		set_page_ext(page, netbk, head_idx);
> +		netbk->mmap_pages[head_idx] = page;
> +		frag_set_pending_idx(&frags[shinfo->nr_frags], head_idx);

> @@ -1548,13 +1649,34 @@ static void xen_netbk_idx_release(struct xen_netbk *netbk, u16 pending_idx,
[...]
> +		/* Setting any number other than
> +		 * INVALID_PENDING_RING_IDX indicates this slot is
> +		 * starting a new packet / ending a previous packet.
> +		 */
> +		pending_tx_info->head = 0;

This doesn't look needed.  It will be initialized again when reusing t
his pending_tx_info again, right?

> +		index = pending_index(netbk->pending_prod++);
> +		netbk->pending_ring[index] = netbk->pending_ring[info_idx];
> +
> +		xenvif_put(vif);
> +
> +		peek = netbk->pending_ring[pending_index(++head)];
> +
> +	} while (netbk->pending_tx_info[peek].head
> +		 == INVALID_PENDING_RING_IDX);
>  
>  	netbk->mmap_pages[pending_idx]->mapping = 0;
>  	put_page(netbk->mmap_pages[pending_idx]);

David

^ permalink raw reply

* Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed
From: Sergei Shtylyov @ 2013-03-25 15:32 UTC (permalink / raw)
  To: David Miller; +Cc: honkiko, netdev, bridge, stephen, herbert, zhiguo.hong
In-Reply-To: <20130324.171537.1255124338630598287.davem@davemloft.net>

Hello.

On 25-03-2013 1:15, David Miller wrote:

>>> When neither IFF_BRIDGE nor IFF_BRIDGE_PORT is set,
>>> and afspec == NULL but  protinfo != NULL, we run into
>>> "if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
>>> random value in ret.

>>> Thanks to Sergei for pointing out the error in commit comments.

>>> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>

>>      For the future, if you post the revised version of the patch, you
>> should indicate in the subject it like this: [PATCH v2].

> I'm disappointed that you have enough energy to point out such a lower
> priority omission, but you lack the time for something more important,
> which is giving this patch your explicit ACK if it fixes all of the
> issues you pointed out to him.

    Sorry, I don't usuallly ACK patches in an area I'm not closely familiar 
with. Though I can add:

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

WBR, Sergei

^ permalink raw reply

* Re: [PATCH v2 1/1]core:Change a wrong explain about dev_get_by_name
From: Sergei Shtylyov @ 2013-03-25 15:44 UTC (permalink / raw)
  To: tingwei liu
  Cc: netdev, Alexey Kuznetsov, davem, Eric Dumazet, Ben Hutchings,
	Linus Torvalds
In-Reply-To: <CA+qZnSTm8kdmrzQLc03Jyk81cBtEpCNHqsT1fFCQ_vstzYcOiw@mail.gmail.com>

Hello.

On 25-03-2013 8:10, tingwei liu wrote:

> From: Tingwei Liu <tingw.liu@gmail.com>
> Date: Mon, 25 Mar 2013 11:09:59 +0800
> Subject: [PATCH] Change a wrong explain about dev_get_by_name

> a long time ago by the  commit.

    Did you mean "before the commit"?

>   commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
>   Author: Linus Torvalds <torvalds@ppc970.osdl.org>
>   Date:   Sat Apr 16 15:20:36 2005 -0700

>      Linux-2.6.12-rc2

>      Initial git repository build. I'm not bothering with the full history,
>      even though we have it. We can create a separate "historical" git
>      archive of that later if we want to, and in the meantime it's about
>      3.2GB when imported into git - space that would just make the early
>      git days unnecessarily complicated, when we don't have a lot of good
>      infrastructure for it.

>      Let it rip!

> When function "list_netdevice" get write lock "dev_base_lock" only
> disable soft interrupt. So dev_get_by_name get read lock
> "dev_base_lock", can not called on interrupt context.

> Signed-off-by: Tingwei Liu <tingw.liu@gmail.com>
> ---
> Changes from v1:
> - add change log
> - add the commnet
>
>   net/core/dev.c |    8 ++++----
>   1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d540ced..20c5c7c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -700,10 +700,10 @@ EXPORT_SYMBOL(dev_get_by_name_rcu);
>   *     @name: name to find
>   *
>   *     Find an interface by name. This can be called from any
> - *     context and does its own locking. The returned handle has
> - *     the usage count incremented and the caller must use dev_put() to
> - *     release it when it is no longer needed. %NULL is returned if no
> - *     matching device is found.
> + *     context except hard irq context and does its own locking.
> + *     The returned handle has the usage count incremented and the
> + *     caller must use dev_put() to release it when it is no longer
> + *     needed. %NULL is returned if no matching device is found.

    It seems your patch has all the tabs replaced by spaces.

WBR, Sergei

^ permalink raw reply

* Re: [Xen-devel] [PATCH 5/6] xen-netback: coalesce slots before copying
From: Wei Liu @ 2013-03-25 15:47 UTC (permalink / raw)
  To: David Vrabel
  Cc: Wei Liu, netdev@vger.kernel.org, annie.li@oracle.com,
	konrad.wilk@oracle.com, Ian Campbell, xen-devel@lists.xen.org
In-Reply-To: <5150699C.6080209@citrix.com>

On Mon, Mar 25, 2013 at 3:13 PM, David Vrabel <david.vrabel@citrix.com> wrote:
> On 25/03/13 11:08, Wei Liu wrote:
>> This patch tries to coalesce tx requests when constructing grant copy
>> structures. It enables netback to deal with situation when frontend's
>> MAX_SKB_FRAGS is larger than backend's MAX_SKB_FRAGS.
>>
>> It defines max_skb_slots, which is a estimation of the maximum number of slots
>> a guest can send, anything bigger than that is considered malicious. Now it is
>> set to 20, which should be enough to accommodate Linux (16 to 19).
>
> This maximum needs to be defined as part of the protocol and added to
> the interface header.
>

No, this is not part of the protocol and not a hard limit. It is
configurable by system administrator.

>> Also change variable name from "frags" to "slots" in netbk_count_requests.
>
> I think this renaming should have been done as a separate patch.
>

The frag -> slot thing only make sense after this patch. If I do this,
I will need to partially reverted what I've done in a previous patch
(like the frag -> slot in comment below you pointed out). :-(

>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -47,11 +47,26 @@
>>  #include <asm/xen/hypercall.h>
>>  #include <asm/xen/page.h>
>>
>> +/*
>> + * This is an estimation of the maximum possible frags a SKB might
>> + * have, anything larger than this is considered malicious. Typically
>> + * Linux has 16 to 19.
>> + */
>
> Do you mean "max possible /slots/" a packet might have?
>

Yes.

>> @@ -968,48 +987,114 @@ static struct gnttab_copy *xen_netbk_get_requests(struct xen_netbk *netbk,
> [...]
>> +                             /* Poison these fields, corresponding
>> +                              * fields for head tx req will be set
>> +                              * to correct values after the loop.
>> +                              */
>> +                             netbk->mmap_pages[pending_idx] = (void *)(~0UL);
>> +                             pending_tx_info[pending_idx].head =
>> +                                     INVALID_PENDING_RING_IDX;
>
> Do you need to poison both values?
>

Just for safety. I have BUG_ON in the release path to check for possible error.

>> +
>> +                             if (unlikely(!first)) {
>
> This isn't unlikely is it?
>

For big packet the chance is 1 in max_skb_slots, so 5% (1/20) in default case.

>> +                                     first = &pending_tx_info[pending_idx];
>> +                                     start_idx = index;
>> +                                     head_idx = pending_idx;
>> +                             }
>
> Instead of setting first here why not move the code below here?
>

Because first->req.size needs to be set to dst_offset, it has to be
here anyway. So I put other fields here as well.

>> +             first->req.offset = 0;
>> +             first->req.size = dst_offset;
>> +             first->head = start_idx;
>> +             set_page_ext(page, netbk, head_idx);
>> +             netbk->mmap_pages[head_idx] = page;
>> +             frag_set_pending_idx(&frags[shinfo->nr_frags], head_idx);
>
>> @@ -1548,13 +1649,34 @@ static void xen_netbk_idx_release(struct xen_netbk *netbk, u16 pending_idx,
> [...]
>> +             /* Setting any number other than
>> +              * INVALID_PENDING_RING_IDX indicates this slot is
>> +              * starting a new packet / ending a previous packet.
>> +              */
>> +             pending_tx_info->head = 0;
>
> This doesn't look needed.  It will be initialized again when reusing t
> his pending_tx_info again, right?
>

Yes, it is needed. Otherwise netback responses to invalid tx_info and
cause netfront to crash before coming into the re-initialization path.


Wei.

^ permalink raw reply

* Re: [PATCH 2/6] xen-netfront: reduce gso_max_size to account for ethernet header
From: Sergei Shtylyov @ 2013-03-25 15:50 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, netdev, ian.campbell, annie.li, konrad.wilk,
	david.vrabel
In-Reply-To: <1364209702-12437-3-git-send-email-wei.liu2@citrix.com>

Hello.

On 25-03-2013 15:08, Wei Liu wrote:

> The maximum packet including ethernet header that can be handled by netfront /
> netback wire format is 65535. Reduce gso_max_size accordingly.

> Drop skb and print warning when skb->len > 65535. This can 1) save the effort
> to send malformed packet to netback, 2) help spotting misconfiguration of
> netfront in the future.

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>   drivers/net/xen-netfront.c |   14 ++++++++++++++
>   1 file changed, 14 insertions(+)

> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 5527663..3ae9dc1 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -547,6 +547,18 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
>   	unsigned int len = skb_headlen(skb);
>   	unsigned long flags;
>
> +	/* Wire format of xen_netif_tx_request only supports skb->len
> +	 * < 64K, because size field in xen_netif_tx_request is
> +	 * uint16_t. If skb->len is too big, drop it and alert user about
> +	 * misconfiguration.
> +	 */
> +	if (unlikely(skb->len >= (uint16_t)(~0))) {

    Such types as 'uint16_t' are intended for userland -- use 'u16' instead. 
Better still, just use 0xffffu.

WBR, Sergei

^ permalink raw reply

* Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed
From: David Miller @ 2013-03-25 16:08 UTC (permalink / raw)
  To: sergei.shtylyov; +Cc: honkiko, netdev, bridge, stephen, herbert, zhiguo.hong
In-Reply-To: <51506DFE.4040405@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Mon, 25 Mar 2013 19:32:14 +0400

>    Sorry, I don't usuallly ACK patches in an area I'm not closely
>    familiar with. Though I can add:

If you feel confident enough to ask for corrections, you better be
ready to ACK the result when your requests have been met.

You can't hold other people accountable yet take none of the
same responsibility yourself.

^ permalink raw reply

* Re: [PATCH net-next] ptp: increase the maximum number of clocks
From: Jiri Benc @ 2013-03-25 16:08 UTC (permalink / raw)
  To: Richard Cochran; +Cc: David Miller, netdev
In-Reply-To: <20130323175915.GA3036@netboy.at.omicron.at>

On Sat, 23 Mar 2013 18:59:15 +0100, Richard Cochran wrote:
> Jiri, do you want to work on making the number of clocks essentially
> unlimited? If not, I can take a look at this, but not right away.

I'm currently thinking of using just /dev/ptp for all PHCs in the
system and specifying the desired network device (ifindex, likely)
by an ioctl as the very first operation. What do you think? If this
is viable, I can try to implement it.

As for netlink, I don't see how to do that without reimplementing the
whole kernel<->user space timer API. The API expects clockid_t which
is either a const (a new constant wouldn't help, we can have multiple
PHCs in the system), or a dynamic value. There is no space for more
dynamic values as far as I can see, all combinations of the three lower
bits are already defined. It seems we're left with file descriptors.

 Jiri

-- 
Jiri Benc

^ permalink raw reply

* Re: [PATCH v2 1/1]core:Change a wrong explain about dev_get_by_name
From: David Miller @ 2013-03-25 16:09 UTC (permalink / raw)
  To: sergei.shtylyov
  Cc: tingw.liu, netdev, kuznet, eric.dumazet, bhutchings, linus971
In-Reply-To: <515070CA.2010701@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Mon, 25 Mar 2013 19:44:10 +0400

>    It seems your patch has all the tabs replaced by spaces.

I think this patch should simply be aborted.  It hasn't addressed any
of Eric Dumazet's concerns, the patch submitter can't even keep the
patch from being corrupted by his email client, and the patch is of
near zero value as far as I'm concerned especially considering how
much work and effort is going into hand holding this patch submitter.

^ permalink raw reply

* RE: [PATCH 3/3] dsa: fix freeing of sparse port allocation
From: David Laight @ 2013-03-25 16:03 UTC (permalink / raw)
  To: Florian Fainelli, davem; +Cc: netdev
In-Reply-To: <1364223820-26051-4-git-send-email-florian@openwrt.org>

> If we have defined a sparse port allocation which is non-contiguous and
> contains gaps, the code freeing port_names will just stop when it
> encouters a first NULL port_names, which is not right, we should iterate
> over all possible number of ports (DSA_MAX_PORTS) until we are done.
...
>  		port_index = 0;
> -		while (pd->chip[i].port_names &&
> -			pd->chip[i].port_names[++port_index])
> -			kfree(pd->chip[i].port_names[port_index]);
> +		while (port_index < DSA_MAX_PORTS) {
> +			if (pd->chip[i].port_names[port_index])
> +				kfree(pd->chip[i].port_names[port_index]);
> +			port_index++;
> +		}

A couple of 'odd' differences:

The old code checked pd->chip[i].port_names (as if it
might be a pointer) that is absent from the new version.
(If it is separately allocated it is leaked).

The new code tests and frees pd->chip[i].port_names[0]
whereas the old code ignored the 0th entry.

These may, or may not, be relevant!

	David

^ permalink raw reply

* Re: [PATCH 00/12] Netfilter updates for net-next
From: David Miller @ 2013-03-25 16:13 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1364213752-3934-1-git-send-email-pablo@netfilter.org>

From: pablo@netfilter.org
Date: Mon, 25 Mar 2013 13:15:40 +0100

> The following patchset contains Netfilter/IPVS updates for
> your net-next tree, they are:
 ...
> You can pull these changes from:
> 
> git://1984.lsi.us.es/nf-next master

Pulled, thanks a lot Pablo.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] net: fec: build fec.c and fec_ptp.c to one module
From: David Miller @ 2013-03-25 16:15 UTC (permalink / raw)
  To: Frank.Li; +Cc: festevam, lznuaa, u.kleine-koenig, netdev, linux-arm-kernel
In-Reply-To: <1364173422-7865-1-git-send-email-Frank.Li@freescale.com>

From: Frank Li <Frank.Li@freescale.com>
Date: Mon, 25 Mar 2013 09:03:42 +0800

> fec_ptp.ko can't run individually
> rename fec.c to fec_main.c
> Build fec.o and fec_ptp.o into one fec.ko
> Remove unnessary EXPORT_SYMBOL in fec_ptp
> 
> Signed-off-by: Frank Li <Frank.Li@freescale.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare}
From: Mike Turquette @ 2013-03-25 16:16 UTC (permalink / raw)
  To: Sekhar Nori, David S. Miller
  Cc: netdev, davinci-linux-open-source, Sekhar Nori
In-Reply-To: <1364203547-2960-3-git-send-email-nsekhar@ti.com>

Quoting Sekhar Nori (2013-03-25 02:25:47)
> Use clk_prepare()/clk_unprepare() in the driver since common
> clock framework needs these to be called before clock is enabled.
> 
> This is in preparation of common clock framework migration
> for DaVinci.
> 
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  drivers/net/ethernet/ti/davinci_emac.c |   19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index b1349b5..436296c 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -350,6 +350,7 @@ struct emac_priv {
>         /*platform specific members*/
>         void (*int_enable) (void);
>         void (*int_disable) (void);
> +       struct clk *clk;
>  };
>  
>  /* EMAC TX Host Error description strings */
> @@ -1870,19 +1871,29 @@ static int davinci_emac_probe(struct platform_device *pdev)
>                 dev_err(&pdev->dev, "failed to get EMAC clock\n");
>                 return -EBUSY;
>         }
> +
> +       rc = clk_prepare(emac_clk);
> +       if (rc) {
> +               dev_err(&pdev->dev, "emac clock prepare failed.\n");
> +               return rc;
> +       }
> +

Is clk_enable ever called for emac_clk here?  I don't see it in the
diff.  clk_prepare and clk_enable should usually be paired, even if
simply calling clk_prepare generates the clock signal that your device
needs.

Also this change only calls clk_prepare at probe-time and clk_unprepare
at remove-time.  Preparing a clock can result in power consumption.
With that in mind should your implementation be more aggressive about
calling clk_{un}prepare?

Regards,
Mike

>         emac_bus_frequency = clk_get_rate(emac_clk);
>  
>         /* TODO: Probe PHY here if possible */
>  
>         ndev = alloc_etherdev(sizeof(struct emac_priv));
> -       if (!ndev)
> -               return -ENOMEM;
> +       if (!ndev) {
> +               rc = -ENOMEM;
> +               goto no_etherdev;
> +       }
>  
>         platform_set_drvdata(pdev, ndev);
>         priv = netdev_priv(ndev);
>         priv->pdev = pdev;
>         priv->ndev = ndev;
>         priv->msg_enable = netif_msg_init(debug_level, DAVINCI_EMAC_DEBUG);
> +       priv->clk = emac_clk;
>  
>         spin_lock_init(&priv->lock);
>  
> @@ -2020,6 +2031,8 @@ no_cpdma_chan:
>         cpdma_ctlr_destroy(priv->dma);
>  no_pdata:
>         free_netdev(ndev);
> +no_etherdev:
> +       clk_unprepare(emac_clk);
>         return rc;
>  }
>  
> @@ -2048,6 +2061,8 @@ static int davinci_emac_remove(struct platform_device *pdev)
>         unregister_netdev(ndev);
>         free_netdev(ndev);
>  
> +       clk_unprepare(priv->clk);
> +
>         return 0;
>  }
>  
> -- 
> 1.7.10.1

^ permalink raw reply

* Re: [PATCH 2/6] xen-netfront: reduce gso_max_size to account for ethernet header
From: David Miller @ 2013-03-25 16:18 UTC (permalink / raw)
  To: wei.liu2
  Cc: xen-devel, netdev, ian.campbell, annie.li, konrad.wilk,
	david.vrabel
In-Reply-To: <1364209702-12437-3-git-send-email-wei.liu2@citrix.com>

From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 25 Mar 2013 11:08:18 +0000

> The maximum packet including ethernet header that can be handled by netfront /
> netback wire format is 65535. Reduce gso_max_size accordingly.
> 
> Drop skb and print warning when skb->len > 65535. This can 1) save the effort
> to send malformed packet to netback, 2) help spotting misconfiguration of
> netfront in the future.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

This is effectively the default already, you don't need to change this
value explicitly.

->gso_max_size is set by default to 65536 and then TCP performs this
calculation:

                xmit_size_goal = ((sk->sk_gso_max_size - 1) -                          
                                  inet_csk(sk)->icsk_af_ops->net_header_len -          
                                  inet_csk(sk)->icsk_ext_hdr_len -                     
                                  tp->tcp_header_len);                                 

thereby making it adhere to your limits just fine.

^ permalink raw reply

* Re: [PATCH 1/6] xen-netfront: remove unused variable `extra'
From: David Miller @ 2013-03-25 16:20 UTC (permalink / raw)
  To: wei.liu2
  Cc: xen-devel, netdev, ian.campbell, annie.li, konrad.wilk,
	david.vrabel
In-Reply-To: <1364209702-12437-2-git-send-email-wei.liu2@citrix.com>

From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 25 Mar 2013 11:08:17 +0000

> This variable is supposed to hold reference to the last extra_info in the
> loop. However there is only type of extra info here and the loop to process
> extra info is missing, so this variable is never used and causes confusion.
> 
> Remove it at the moment. We can add it back when necessary.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Applied.

^ permalink raw reply

* Re: [PATCH 3/6] xen-netfront: frags -> slots in xennet_get_responses
From: David Miller @ 2013-03-25 16:20 UTC (permalink / raw)
  To: wei.liu2
  Cc: xen-devel, netdev, ian.campbell, annie.li, konrad.wilk,
	david.vrabel
In-Reply-To: <1364209702-12437-4-git-send-email-wei.liu2@citrix.com>

From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 25 Mar 2013 11:08:19 +0000

> This function is in fact counting the ring slots required for responses.
> Separate the concepts of ring slots and skb frags make the code clearer, as
> now netfront and netback can have different MAX_SKB_FRAGS, slot and frag are
> not mapped 1:1 any more.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Applied.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox