Skip to contentSkip to conversation

can: gw: Netlink related fixes

PATCH Applied Thomas Graf 3 participants 13 messages Jul 5, 2012 – Jul 10, 2012

Subsystems (2)
CAN network drivers CAN network layer
lore
Share & tools
lore

Thomas Graf

author
#0 lore raw

cover can: gw: Netlink related fixes

Hi

These patches fix some Netlink quirks in the CAN gateway code. Unfortunately I do not own any CAN hardware so this is compile tested only. I'm fairly confident though as the changes are mostly straight forward. I would appreciate if someone could run them through a test suite or something though.

Thomas Graf (4):
can: Don't bump nlmsg_len manually
can: Use nla_policy to validate netlink attributes
can: Properly fill the netlink header when responding to RTM_GETROUTE
can: Remove pointless casts

 net/can/gw.c |   90 +++++++++++++++++++++------------------------------------
 1 files changed, 33 insertions(+), 57 deletions(-)
Signature · 1 line
1.7.7.6
author
#1 lore raw

PATCH 1/4 can: Don't bump nlmsg_len manually

nlmsg_end() will take care of this when we finalize the message.

Signed-off-by Thomas Graf <[email protected]>
---
 net/can/gw.c |   27 +--------------------------
 1 files changed, 1 insertions(+), 26 deletions(-)
net/can/gw.c +1 -26
index b41acf2..a3ff980 100644
@@ -462,15 +462,11 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
 	if (gwj->handled_frames) {
 		if (nla_put_u32(skb, CGW_HANDLED, gwj->handled_frames) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(u32));
 	}
 
 	if (gwj->dropped_frames) {
 		if (nla_put_u32(skb, CGW_DROPPED, gwj->dropped_frames) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(u32));
 	}
 
 	/* check non default settings of attributes */

@@ -480,8 +476,6 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
 		mb.modtype = gwj->mod.modtype.and;
 		if (nla_put(skb, CGW_MOD_AND, sizeof(mb), &mb) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(mb));
 	}
 
 	if (gwj->mod.modtype.or) {

@@ -489,8 +483,6 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
 		mb.modtype = gwj->mod.modtype.or;
 		if (nla_put(skb, CGW_MOD_OR, sizeof(mb), &mb) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(mb));
 	}
 
 	if (gwj->mod.modtype.xor) {

@@ -498,8 +490,6 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
 		mb.modtype = gwj->mod.modtype.xor;
 		if (nla_put(skb, CGW_MOD_XOR, sizeof(mb), &mb) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(mb));
 	}
 
 	if (gwj->mod.modtype.set) {

@@ -507,26 +497,18 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
 		mb.modtype = gwj->mod.modtype.set;
 		if (nla_put(skb, CGW_MOD_SET, sizeof(mb), &mb) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(mb));
 	}
 
 	if (gwj->mod.csumfunc.crc8) {
 		if (nla_put(skb, CGW_CS_CRC8, CGW_CS_CRC8_LEN,
 			    &gwj->mod.csum.crc8) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + \
-				NLA_ALIGN(CGW_CS_CRC8_LEN);
 	}
 
 	if (gwj->mod.csumfunc.xor) {
 		if (nla_put(skb, CGW_CS_XOR, CGW_CS_XOR_LEN,
 			    &gwj->mod.csum.xor) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + \
-				NLA_ALIGN(CGW_CS_XOR_LEN);
 	}
 
 	if (gwj->gwtype == CGW_TYPE_CAN_CAN) {

@@ -535,23 +517,16 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
 			if (nla_put(skb, CGW_FILTER, sizeof(struct can_filter),
 				    &gwj->ccgw.filter) < 0)
 				goto cancel;
-			else
-				nlh->nlmsg_len += NLA_HDRLEN +
-					NLA_ALIGN(sizeof(struct can_filter));
 		}
 
 		if (nla_put_u32(skb, CGW_SRC_IF, gwj->ccgw.src_idx) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(u32));
 
 		if (nla_put_u32(skb, CGW_DST_IF, gwj->ccgw.dst_idx) < 0)
 			goto cancel;
-		else
-			nlh->nlmsg_len += NLA_HDRLEN + NLA_ALIGN(sizeof(u32));
 	}
 
-	return skb->len;
+	return nlmsg_end(skb, nlh);
 
 cancel:
 	nlmsg_cancel(skb, nlh);
Signature · 1 line
1.7.7.6
author
#2 lore raw

PATCH 2/4 can: Use nla_policy to validate netlink attributes

In reply to Thomas Graf

Also use nla_get_u32() instead of nla_memcpy() to access u32 attribtues.

Signed-off-by Thomas Graf <[email protected]>
---
 net/can/gw.c |   47 +++++++++++++++++++++++------------------------
 1 files changed, 23 insertions(+), 24 deletions(-)
net/can/gw.c +23 -24
index a3ff980..a1c639c 100644
@@ -558,6 +558,18 @@ cont:
 	return skb->len;
 }
 
+static const struct nla_policy cgw_policy[CGW_MAX+1] = {
+	[CGW_MOD_AND]	= { .len = sizeof(struct cgw_frame_mod) },
+	[CGW_MOD_OR]	= { .len = sizeof(struct cgw_frame_mod) },
+	[CGW_MOD_XOR]	= { .len = sizeof(struct cgw_frame_mod) },
+	[CGW_MOD_SET]	= { .len = sizeof(struct cgw_frame_mod) },
+	[CGW_CS_XOR]	= { .len = sizeof(struct cgw_csum_xor) },
+	[CGW_CS_CRC8]	= { .len = sizeof(struct cgw_csum_crc8) },
+	[CGW_SRC_IF]	= { .type = NLA_U32 },
+	[CGW_DST_IF]	= { .type = NLA_U32 },
+	[CGW_FILTER]	= { .len = sizeof(struct can_filter) },
+};
+
 /* check for common and gwtype specific attributes */
 static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 			  u8 gwtype, void *gwtypeattr)

@@ -570,14 +582,14 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 	/* initialize modification & checksum data space */
 	memset(mod, 0, sizeof(*mod));
 
-	err = nlmsg_parse(nlh, sizeof(struct rtcanmsg), tb, CGW_MAX, NULL);
+	err = nlmsg_parse(nlh, sizeof(struct rtcanmsg), tb, CGW_MAX,
+			  cgw_policy);
 	if (err < 0)
 		return err;
 
 	/* check for AND/OR/XOR/SET modifications */
 
-	if (tb[CGW_MOD_AND] &&
-	    nla_len(tb[CGW_MOD_AND]) == CGW_MODATTR_LEN) {
+	if (tb[CGW_MOD_AND]) {
 		nla_memcpy(&mb, tb[CGW_MOD_AND], CGW_MODATTR_LEN);
 
 		canframecpy(&mod->modframe.and, &mb.cf);

@@ -593,8 +605,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 			mod->modfunc[modidx++] = mod_and_data;
 	}
 
-	if (tb[CGW_MOD_OR] &&
-	    nla_len(tb[CGW_MOD_OR]) == CGW_MODATTR_LEN) {
+	if (tb[CGW_MOD_OR]) {
 		nla_memcpy(&mb, tb[CGW_MOD_OR], CGW_MODATTR_LEN);
 
 		canframecpy(&mod->modframe.or, &mb.cf);

@@ -610,8 +621,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 			mod->modfunc[modidx++] = mod_or_data;
 	}
 
-	if (tb[CGW_MOD_XOR] &&
-	    nla_len(tb[CGW_MOD_XOR]) == CGW_MODATTR_LEN) {
+	if (tb[CGW_MOD_XOR]) {
 		nla_memcpy(&mb, tb[CGW_MOD_XOR], CGW_MODATTR_LEN);
 
 		canframecpy(&mod->modframe.xor, &mb.cf);

@@ -627,8 +637,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 			mod->modfunc[modidx++] = mod_xor_data;
 	}
 
-	if (tb[CGW_MOD_SET] &&
-	    nla_len(tb[CGW_MOD_SET]) == CGW_MODATTR_LEN) {
+	if (tb[CGW_MOD_SET]) {
 		nla_memcpy(&mb, tb[CGW_MOD_SET], CGW_MODATTR_LEN);
 
 		canframecpy(&mod->modframe.set, &mb.cf);

@@ -647,9 +656,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 	/* check for checksum operations after CAN frame modifications */
 	if (modidx) {
 
-		if (tb[CGW_CS_CRC8] &&
-		    nla_len(tb[CGW_CS_CRC8]) == CGW_CS_CRC8_LEN) {
-
+		if (tb[CGW_CS_CRC8]) {
 			struct cgw_csum_crc8 *c = (struct cgw_csum_crc8 *)\
 				nla_data(tb[CGW_CS_CRC8]);
 

@@ -674,9 +681,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 				mod->csumfunc.crc8 = cgw_csum_crc8_neg;
 		}
 
-		if (tb[CGW_CS_XOR] &&
-		    nla_len(tb[CGW_CS_XOR]) == CGW_CS_XOR_LEN) {
-
+		if (tb[CGW_CS_XOR]) {
 			struct cgw_csum_xor *c = (struct cgw_csum_xor *)\
 				nla_data(tb[CGW_CS_XOR]);
 

@@ -710,8 +715,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 		memset(ccgw, 0, sizeof(*ccgw));
 
 		/* check for can_filter in attributes */
-		if (tb[CGW_FILTER] &&
-		    nla_len(tb[CGW_FILTER]) == sizeof(struct can_filter))
+		if (tb[CGW_FILTER])
 			nla_memcpy(&ccgw->filter, tb[CGW_FILTER],
 				   sizeof(struct can_filter));
 

@@ -721,13 +725,8 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 		if (!tb[CGW_SRC_IF] || !tb[CGW_DST_IF])
 			return err;
 
-		if (nla_len(tb[CGW_SRC_IF]) == sizeof(u32))
-			nla_memcpy(&ccgw->src_idx, tb[CGW_SRC_IF],
-				   sizeof(u32));
-
-		if (nla_len(tb[CGW_DST_IF]) == sizeof(u32))
-			nla_memcpy(&ccgw->dst_idx, tb[CGW_DST_IF],
-				   sizeof(u32));
+		ccgw->src_idx = nla_get_u32(tb[CGW_SRC_IF]);
+		ccgw->dst_idx = nla_get_u32(tb[CGW_DST_IF]);
 
 		/* both indices set to 0 for flushing all routing entries */
 		if (!ccgw->src_idx && !ccgw->dst_idx)
Signature · 1 line
1.7.7.6
author
#3 lore raw

PATCH 3/4 can: Properly fill the netlink header when responding to RTM_GETROUTE

In reply to Thomas Graf

- set message type to RTM_NEWROUTE
- relate to original request by inheriting the sequence and port number.
- set NLM_F_MULTI because it's a dump and more messages will follow

Signed-off-by Thomas Graf <[email protected]>
---
 net/can/gw.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
net/can/gw.c +7 -3
index a1c639c..20c36e1 100644
@@ -444,11 +444,14 @@ static int cgw_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
+static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
+		       u32 pid, u32 seq, int flags)
 {
 	struct cgw_frame_mod mb;
 	struct rtcanmsg *rtcan;
-	struct nlmsghdr *nlh = nlmsg_put(skb, 0, 0, 0, sizeof(*rtcan), 0);
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*rtcan), flags);
 	if (!nlh)
 		return -EMSGSIZE;
 

@@ -546,7 +549,8 @@ static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb)
 		if (idx < s_idx)
 			goto cont;
 
-		if (cgw_put_job(skb, gwj) < 0)
+		if (cgw_put_job(skb, gwj, RTM_NEWROUTE, NETLINK_CB(cb->skb).pid,
+		    cb->nlh->nlmsg_seq, NLM_F_MULTI) < 0)
 			break;
 cont:
 		idx++;
Signature · 1 line
1.7.7.6
maintainer
#4 lore raw

Re: [PATCH 3/4] can: Properly fill the netlink header when responding to RTM_GETROUTE

Quoted 31 lines · Thomas Graf

- set message type to RTM_NEWROUTE
- relate to original request by inheriting the sequence and port number.
- set NLM_F_MULTI because it's a dump and more messages will follow

Signed-off-by Thomas Graf <[email protected]>
---
 net/can/gw.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
net/can/gw.c +5 -2
index a1c639c..20c36e1 100644
@@ -444,11 +444,14 @@ static int cgw_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
+static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
+		       u32 pid, u32 seq, int flags)
 {
 	struct cgw_frame_mod mb;
 	struct rtcanmsg *rtcan;
-	struct nlmsghdr *nlh = nlmsg_put(skb, 0, 0, 0, sizeof(*rtcan), 0);
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*rtcan), flags);
 	if (!nlh)
 		return -EMSGSIZE;

net/can/gw.c static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb) · line 556

Quoted 9 lines
net/can/gw.c +2 -1
@@ -546,7 +549,8 @@ static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb)
 		if (idx < s_idx)
 			goto cont;
 
-		if (cgw_put_job(skb, gwj) < 0)
+		if (cgw_put_job(skb, gwj, RTM_NEWROUTE, NETLINK_CB(cb->skb).pid,
+		    cb->nlh->nlmsg_seq, NLM_F_MULTI) < 0)
 			break;
 cont:
 		idx++;

Hello Thomas,

what about this approach which reduces the number of parameters and stack space when calling cgw_put_job() ??

net/can/gw.c +7 -3
index b41acf2..b6b9e0a 100644
@@ -444,11 +444,15 @@ static int cgw_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
+static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj,
+		       struct netlink_callback *cb)
 {
 	struct cgw_frame_mod mb;
 	struct rtcanmsg *rtcan;
-	struct nlmsghdr *nlh = nlmsg_put(skb, 0, 0, 0, sizeof(*rtcan), 0);
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq,
+			RTM_NEWROUTE, sizeof(*rtcan), NLM_F_MULTI);
 	if (!nlh)
 		return -EMSGSIZE;
 

@@ -571,7 +575,7 @@ static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb)
 		if (idx < s_idx)
 			goto cont;
 
-		if (cgw_put_job(skb, gwj) < 0)
+		if (cgw_put_job(skb, gwj, cb) < 0)
 			break;
 cont:
 		idx++;

Regards,
Oliver

author
#5 lore raw

Re: [PATCH 3/4] can: Properly fill the netlink header when responding to RTM_GETROUTE

Quoted 27 lines · Oliver Hartkopp

what about this approach which reduces the number of parameters and stack space when calling cgw_put_job() ??

net/can/gw.c +6 -2
index b41acf2..b6b9e0a 100644
@@ -444,11 +444,15 @@ static int cgw_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
+static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj,
+		       struct netlink_callback *cb)
 {
 	struct cgw_frame_mod mb;
 	struct rtcanmsg *rtcan;
-	struct nlmsghdr *nlh = nlmsg_put(skb, 0, 0, 0, sizeof(*rtcan), 0);
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq,
+			RTM_NEWROUTE, sizeof(*rtcan), NLM_F_MULTI);
 	if (!nlh)
 		return -EMSGSIZE;

net/can/gw.c static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb) · line 581

Quoted 8 lines
net/can/gw.c +1 -1
@@ -571,7 +575,7 @@ static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb)
 		if (idx < s_idx)
 			goto cont;
 
-		if (cgw_put_job(skb, gwj) < 0)
+		if (cgw_put_job(skb, gwj, cb) < 0)
 			break;
 cont:
 		idx++;

The format I've chosen allows to implement a non-dump RTM_GETROUTE using cgw_put_job() if needed and is in line with how such filling functions are typically prototyped in other netlink protocol implementations.

~Thomas

maintainer
#6 lore raw

Re: [PATCH 3/4] can: Properly fill the netlink header when responding to RTM_GETROUTE

Quoted 31 lines · Thomas Graf
Quoted 30 lines · Oliver Hartkopp

what about this approach which reduces the number of parameters and stack space when calling cgw_put_job() ??

net/can/gw.c +6 -2
index b41acf2..b6b9e0a 100644
@@ -444,11 +444,15 @@ static int cgw_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
+static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj,
+		       struct netlink_callback *cb)
 {
 	struct cgw_frame_mod mb;
 	struct rtcanmsg *rtcan;
-	struct nlmsghdr *nlh = nlmsg_put(skb, 0, 0, 0, sizeof(*rtcan), 0);
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq,
+			RTM_NEWROUTE, sizeof(*rtcan), NLM_F_MULTI);
 	if (!nlh)
 		return -EMSGSIZE;
 

@@ -571,7 +575,7 @@ static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb)
 		if (idx < s_idx)
 			goto cont;
Quoted 10 lines

net/can/gw.c static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb) · line 581

Quoted 5 lines
net/can/gw.c +1 -1
-		if (cgw_put_job(skb, gwj) < 0)
+		if (cgw_put_job(skb, gwj, cb) < 0)
 			break;
 cont:
 		idx++;

The format I've chosen allows to implement a non-dump RTM_GETROUTE using cgw_put_job() if needed and is in line with how such filling functions are typically prototyped in other netlink protocol implementations.

Yes, i also checked that the format was commonly used, but i did not find any use-case for cgw_put_job() besides the full dump ...

Btw. both approaches are fine to me then.

Many thanks,
Oliver

maintainer Tested-by Acked-by on v1 3/4
#7 lore raw

Re: [PATCH 3/4] can: Properly fill the netlink header when responding to RTM_GETROUTE

In reply to Thomas Graf

Hello Thomas,

besides the fact that i would like to simplify this patch (3/4) as shown below, i was able to test it now.

You may add my

Tested-by Oliver Hartkopp <[email protected]>
Acked-by Oliver Hartkopp <[email protected]>

to the next post.

Do you have an URL to pull this patch set for Marc?

Thanks for your effort!

Best regards,
Oliver

Quoted 31 lines · Thomas Graf

- set message type to RTM_NEWROUTE
- relate to original request by inheriting the sequence and port number.
- set NLM_F_MULTI because it's a dump and more messages will follow

Signed-off-by Thomas Graf <[email protected]>
---
 net/can/gw.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
net/can/gw.c +5 -2
index a1c639c..20c36e1 100644
@@ -444,11 +444,14 @@ static int cgw_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
+static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
+		       u32 pid, u32 seq, int flags)
 {
 	struct cgw_frame_mod mb;
 	struct rtcanmsg *rtcan;
-	struct nlmsghdr *nlh = nlmsg_put(skb, 0, 0, 0, sizeof(*rtcan), 0);
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*rtcan), flags);
 	if (!nlh)
 		return -EMSGSIZE;

net/can/gw.c static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb) · line 556

Quoted 9 lines
net/can/gw.c +2 -1
@@ -546,7 +549,8 @@ static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb)
 		if (idx < s_idx)
 			goto cont;
 
-		if (cgw_put_job(skb, gwj) < 0)
+		if (cgw_put_job(skb, gwj, RTM_NEWROUTE, NETLINK_CB(cb->skb).pid,
+		    cb->nlh->nlmsg_seq, NLM_F_MULTI) < 0)
 			break;
 cont:
 		idx++;

Hello Thomas,

what about this approach which reduces the number of parameters and stack space when calling cgw_put_job() ??

net/can/gw.c +7 -3
index b41acf2..b6b9e0a 100644
@@ -444,11 +444,15 @@ static int cgw_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
-static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj)
+static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj,
+		       struct netlink_callback *cb)
 {
 	struct cgw_frame_mod mb;
 	struct rtcanmsg *rtcan;
-	struct nlmsghdr *nlh = nlmsg_put(skb, 0, 0, 0, sizeof(*rtcan), 0);
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq,
+			RTM_NEWROUTE, sizeof(*rtcan), NLM_F_MULTI);
 	if (!nlh)
 		return -EMSGSIZE;

@@ -571,7 +575,7 @@ static int cgw_dump_jobs(struct sk_buff *skb, struct netlink_callback *cb)
 		if (idx < s_idx)
 			goto cont;
-		if (cgw_put_job(skb, gwj) < 0)
+		if (cgw_put_job(skb, gwj, cb) < 0)
 			break;
 cont:
 		idx++;

Regards,
Oliver

maintainer
#8 lore raw

Re: [PATCH 3/4] can: Properly fill the netlink header when responding to RTM_GETROUTE

Quoted 5 lines · Oliver Hartkopp

Hello Thomas,

besides the fact that i would like to simplify this patch (3/4) as shown below, i was able to test it now.

Quoted 8 lines

You may add my

Tested-by Oliver Hartkopp <[email protected]>
Acked-by Oliver Hartkopp <[email protected]>

to the next post.

Do you have an URL to pull this patch set for Marc?

I can take this series as is and add your Tested-by and Acked-by to each patch. Is this okay?

Marc

Signature · 4 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
maintainer
#9 lore raw

Re: [PATCH 3/4] can: Properly fill the netlink header when responding to RTM_GETROUTE

Quoted 11 lines · Marc Kleine-Budde
Quoted 10 lines · Oliver Hartkopp

Hello Thomas,

besides the fact that i would like to simplify this patch (3/4) as shown below, i was able to test it now.

You may add my

Tested-by Oliver Hartkopp <[email protected]>
Acked-by Oliver Hartkopp <[email protected]>
Quoted 6 lines
Quoted 3 lines

to the next post.

Do you have an URL to pull this patch set for Marc?

I can take this series as is and add your Tested-by and Acked-by to each patch. Is this okay?

Yes. That would be fine!

Tnx,
Oliver

author
#10 lore raw

PATCH 4/4 can: Remove pointless casts

In reply to Thomas Graf

No need to cast return value of nla_data()

Signed-off-by Thomas Graf <[email protected]>
---
 net/can/gw.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
net/can/gw.c +2 -4
index 20c36e1..b54d5e6 100644
@@ -661,8 +661,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 	if (modidx) {
 
 		if (tb[CGW_CS_CRC8]) {
-			struct cgw_csum_crc8 *c = (struct cgw_csum_crc8 *)\
-				nla_data(tb[CGW_CS_CRC8]);
+			struct cgw_csum_crc8 *c = nla_data(tb[CGW_CS_CRC8]);
 
 			err = cgw_chk_csum_parms(c->from_idx, c->to_idx,
 						 c->result_idx);

@@ -686,8 +685,7 @@ static int cgw_parse_attr(struct nlmsghdr *nlh, struct cf_mod *mod,
 		}
 
 		if (tb[CGW_CS_XOR]) {
-			struct cgw_csum_xor *c = (struct cgw_csum_xor *)\
-				nla_data(tb[CGW_CS_XOR]);
+			struct cgw_csum_xor *c = nla_data(tb[CGW_CS_XOR]);
 
 			err = cgw_chk_csum_parms(c->from_idx, c->to_idx,
 						 c->result_idx);
Signature · 1 line
1.7.7.6
maintainer
#11 lore raw
In reply to Thomas Graf
Quoted 7 lines · Thomas Graf

These patches fix some Netlink quirks in the CAN gateway code. Unfortunately I do not own any CAN hardware so this is compile tested only. I'm fairly confident though as the changes are mostly straight forward. I would appreciate if someone could run them through a test suite or something though.

Quoted 9 lines

Thomas Graf (4):
can: Don't bump nlmsg_len manually
can: Use nla_policy to validate netlink attributes
can: Properly fill the netlink header when responding to RTM_GETROUTE
can: Remove pointless casts

 net/can/gw.c |   90 +++++++++++++++++++++------------------------------------
 1 files changed, 33 insertions(+), 57 deletions(-)

Hi Thomas,

thanks for the cleanups!

The can-gw was my first netlink based project and i did the best i knew.

From the first sight it looks good. I now also got behind the nla_policy trick, which checks the minimal length too.

I'll test the patches this weekend with the virtual CAN interfaces (no CAN HW needed), so that we probably catch the current development window.

Best regards,
Oliver

maintainer Applied
#12 lore raw
In reply to Thomas Graf
Quoted 9 lines · Thomas Graf

Hi

These patches fix some Netlink quirks in the CAN gateway code. Unfortunately I do not own any CAN hardware so this is compile tested only. I'm fairly confident though as the changes are mostly straight forward. I would appreciate if someone could run them through a test suite or something though.

Quoted 5 lines

Thomas Graf (4):
can: Don't bump nlmsg_len manually
can: Use nla_policy to validate netlink attributes
can: Properly fill the netlink header when responding to RTM_GETROUTE
can: Remove pointless casts

Thanks applied to linux-can-next.

Thanks, Marc

Signature · 4 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |