diff mbox

[net-next,3/4] team: add binary option type

Message ID 1333227682-10235-4-git-send-email-jpirko@redhat.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Jiri Pirko March 31, 2012, 9:01 p.m. UTC
For transfering generic binary data (e.g. BPF code), introduce new
binary option type.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/team/team.c |   28 ++++++++++++++++++++++++----
 include/linux/if_team.h |    8 ++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

Comments

David Miller April 3, 2012, 10:38 p.m. UTC | #1
From: Jiri Pirko <jpirko@redhat.com>
Date: Sat, 31 Mar 2012 23:01:21 +0200

> For transfering generic binary data (e.g. BPF code), introduce new
> binary option type.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Several issues:

> +		struct team_option_binary tbinary;

You put this into a netlink attribute, it has a non-fixed
type size because it uses pointer.  A compat task will do
the wrong thing and you won't interpret it's attribute
correctly.

> +			NLA_PUT_U8(skb, TEAM_ATTR_OPTION_TYPE, NLA_BINARY);

net-next no longer has NLA_PUT*(), so you'll need to adjust
this as well.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Pirko April 4, 2012, 12:29 p.m. UTC | #2
Wed, Apr 04, 2012 at 12:38:16AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jpirko@redhat.com>
>Date: Sat, 31 Mar 2012 23:01:21 +0200
>
>> For transfering generic binary data (e.g. BPF code), introduce new
>> binary option type.
>> 
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>
>Several issues:
>
>> +		struct team_option_binary tbinary;
>
>You put this into a netlink attribute, it has a non-fixed
>type size because it uses pointer.  A compat task will do
>the wrong thing and you won't interpret it's attribute
>correctly.

I'm not nla_putting struct team_option_binary tbinary. I'm putting only
the data on what the pointer stored into that points (tbinary.data):

nla_put(skb, TEAM_ATTR_OPTION_DATA, tbinary.data_len, tbinary.data));


>
>> +			NLA_PUT_U8(skb, TEAM_ATTR_OPTION_TYPE, NLA_BINARY);
>
>net-next no longer has NLA_PUT*(), so you'll need to adjust
>this as well.


Sure I'll change this.

Jirka
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 4, 2012, 9:45 p.m. UTC | #3
From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 4 Apr 2012 14:29:31 +0200

> Wed, Apr 04, 2012 at 12:38:16AM CEST, davem@davemloft.net wrote:
>>From: Jiri Pirko <jpirko@redhat.com>
>>Date: Sat, 31 Mar 2012 23:01:21 +0200
>>
>>> For transfering generic binary data (e.g. BPF code), introduce new
>>> binary option type.
>>> 
>>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>
>>Several issues:
>>
>>> +		struct team_option_binary tbinary;
>>
>>You put this into a netlink attribute, it has a non-fixed
>>type size because it uses pointer.  A compat task will do
>>the wrong thing and you won't interpret it's attribute
>>correctly.
> 
> I'm not nla_putting struct team_option_binary tbinary. I'm putting only
> the data on what the pointer stored into that points (tbinary.data):
> 
> nla_put(skb, TEAM_ATTR_OPTION_DATA, tbinary.data_len, tbinary.data));

Aha, I misread, thanks for explaining.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Pirko April 4, 2012, 10:14 p.m. UTC | #4
Wed, Apr 04, 2012 at 11:45:47PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jpirko@redhat.com>
>Date: Wed, 4 Apr 2012 14:29:31 +0200
>
>> Wed, Apr 04, 2012 at 12:38:16AM CEST, davem@davemloft.net wrote:
>>>From: Jiri Pirko <jpirko@redhat.com>
>>>Date: Sat, 31 Mar 2012 23:01:21 +0200
>>>
>>>> For transfering generic binary data (e.g. BPF code), introduce new
>>>> binary option type.
>>>> 
>>>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>>
>>>Several issues:
>>>
>>>> +		struct team_option_binary tbinary;
>>>
>>>You put this into a netlink attribute, it has a non-fixed
>>>type size because it uses pointer.  A compat task will do
>>>the wrong thing and you won't interpret it's attribute
>>>correctly.
>> 
>> I'm not nla_putting struct team_option_binary tbinary. I'm putting only
>> the data on what the pointer stored into that points (tbinary.data):
>> 
>> nla_put(skb, TEAM_ATTR_OPTION_DATA, tbinary.data_len, tbinary.data));
>
>Aha, I misread, thanks for explaining.

Going to repost 3/4 and 4/4.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 8f81805..9ad52b5b 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1145,10 +1145,7 @@  team_nl_option_policy[TEAM_ATTR_OPTION_MAX + 1] = {
 	},
 	[TEAM_ATTR_OPTION_CHANGED]		= { .type = NLA_FLAG },
 	[TEAM_ATTR_OPTION_TYPE]			= { .type = NLA_U8 },
-	[TEAM_ATTR_OPTION_DATA] = {
-		.type = NLA_BINARY,
-		.len = TEAM_STRING_MAX_LEN,
-	},
+	[TEAM_ATTR_OPTION_DATA]			= { .type = NLA_BINARY },
 };
 
 static int team_nl_cmd_noop(struct sk_buff *skb, struct genl_info *info)
@@ -1256,6 +1253,7 @@  static int team_nl_fill_options_get(struct sk_buff *skb,
 	list_for_each_entry(option, &team->option_list, list) {
 		struct nlattr *option_item;
 		long arg;
+		struct team_option_binary tbinary;
 
 		/* Include only changed options if fill all mode is not on */
 		if (!fillall && !option->changed)
@@ -1282,6 +1280,13 @@  static int team_nl_fill_options_get(struct sk_buff *skb,
 			NLA_PUT_STRING(skb, TEAM_ATTR_OPTION_DATA,
 				       (char *) arg);
 			break;
+		case TEAM_OPTION_TYPE_BINARY:
+			NLA_PUT_U8(skb, TEAM_ATTR_OPTION_TYPE, NLA_BINARY);
+			arg = (long) &tbinary;
+			team_option_get(team, option, &arg);
+			NLA_PUT(skb, TEAM_ATTR_OPTION_DATA,
+				tbinary.data_len, tbinary.data);
+			break;
 		default:
 			BUG();
 		}
@@ -1366,6 +1371,9 @@  static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
 		case NLA_STRING:
 			opt_type = TEAM_OPTION_TYPE_STRING;
 			break;
+		case NLA_BINARY:
+			opt_type = TEAM_OPTION_TYPE_BINARY;
+			break;
 		default:
 			goto team_put;
 		}
@@ -1374,19 +1382,31 @@  static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
 		list_for_each_entry(option, &team->option_list, list) {
 			long arg;
 			struct nlattr *opt_data_attr;
+			struct team_option_binary tbinary;
+			int data_len;
 
 			if (option->type != opt_type ||
 			    strcmp(option->name, opt_name))
 				continue;
 			opt_found = true;
 			opt_data_attr = mode_attrs[TEAM_ATTR_OPTION_DATA];
+			data_len = nla_len(opt_data_attr);
 			switch (opt_type) {
 			case TEAM_OPTION_TYPE_U32:
 				arg = nla_get_u32(opt_data_attr);
 				break;
 			case TEAM_OPTION_TYPE_STRING:
+				if (data_len > TEAM_STRING_MAX_LEN) {
+					err = -EINVAL;
+					goto team_put;
+				}
 				arg = (long) nla_data(opt_data_attr);
 				break;
+			case TEAM_OPTION_TYPE_BINARY:
+				tbinary.data_len = data_len;
+				tbinary.data = nla_data(opt_data_attr);
+				arg = (long) &tbinary;
+				break;
 			default:
 				BUG();
 			}
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 58404b0..41163ac 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -68,6 +68,7 @@  struct team_mode_ops {
 enum team_option_type {
 	TEAM_OPTION_TYPE_U32,
 	TEAM_OPTION_TYPE_STRING,
+	TEAM_OPTION_TYPE_BINARY,
 };
 
 struct team_option {
@@ -82,6 +83,13 @@  struct team_option {
 	bool removed;
 };
 
+struct team_option_binary {
+	u32 data_len;
+	void *data;
+};
+
+#define team_optarg_tbinary(arg) (*((struct team_option_binary **) arg))
+
 struct team_mode {
 	struct list_head list;
 	const char *kind;