* [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
@ 2014-12-31 16:48 roopa
2014-12-31 17:45 ` Jeremiah Mahler
0 siblings, 1 reply; 7+ messages in thread
From: roopa @ 2014-12-31 16:48 UTC (permalink / raw)
To: netdev, hemminger, vyasevic; +Cc: sfeldma, wkok, Roopa Prabhu
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds (as suggested by scott feldman),
- new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
vlan list
- And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
include/uapi/linux/if_bridge.h | 4 ++++
net/bridge/br_netlink.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index b03ee8f..fa468aa 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -112,12 +112,14 @@ struct __fdb_entry {
* [IFLA_BRIDGE_FLAGS]
* [IFLA_BRIDGE_MODE]
* [IFLA_BRIDGE_VLAN_INFO]
+ * [IFLA_BRIDGE_VLAN_INFO_LIST]
* }
*/
enum {
IFLA_BRIDGE_FLAGS,
IFLA_BRIDGE_MODE,
IFLA_BRIDGE_VLAN_INFO,
+ IFLA_BRIDGE_VLAN_INFO_LIST,
__IFLA_BRIDGE_MAX,
};
#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
@@ -125,6 +127,8 @@ enum {
#define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
#define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
#define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
+#define BRIDGE_VLAN_INFO_RANGE_START (1<<3) /* VLAN is start of vlan range */
+#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
struct bridge_vlan_info {
__u16 flags;
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9f5eb55..492ef6a 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
[IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
[IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
.len = sizeof(struct bridge_vlan_info), },
+ [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
};
static int br_afspec(struct net_bridge *br,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
2014-12-31 16:48 [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges roopa
@ 2014-12-31 17:45 ` Jeremiah Mahler
2014-12-31 18:15 ` roopa
0 siblings, 1 reply; 7+ messages in thread
From: Jeremiah Mahler @ 2014-12-31 17:45 UTC (permalink / raw)
To: roopa; +Cc: netdev, hemminger, vyasevic, sfeldma, wkok
Roopa,
On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch adds (as suggested by scott feldman),
> - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
> vlan list
> - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
> BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> include/uapi/linux/if_bridge.h | 4 ++++
> net/bridge/br_netlink.c | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index b03ee8f..fa468aa 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -112,12 +112,14 @@ struct __fdb_entry {
> * [IFLA_BRIDGE_FLAGS]
> * [IFLA_BRIDGE_MODE]
> * [IFLA_BRIDGE_VLAN_INFO]
> + * [IFLA_BRIDGE_VLAN_INFO_LIST]
> * }
> */
> enum {
> IFLA_BRIDGE_FLAGS,
> IFLA_BRIDGE_MODE,
> IFLA_BRIDGE_VLAN_INFO,
> + IFLA_BRIDGE_VLAN_INFO_LIST,
> __IFLA_BRIDGE_MAX,
> };
> #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
> @@ -125,6 +127,8 @@ enum {
> #define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
> #define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
> #define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
> +#define BRIDGE_VLAN_INFO_RANGE_START (1<<3) /* VLAN is start of vlan range */
> +#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
>
You add these here but you don't use them until the next patch.
If they were wrong a bisect would point to the next patch.
I would add them in the next patch where you start to use them.
> struct bridge_vlan_info {
> __u16 flags;
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 9f5eb55..492ef6a 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
> [IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
> [IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
> .len = sizeof(struct bridge_vlan_info), },
> + [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
> };
>
> static int br_afspec(struct net_bridge *br,
> --
> 1.7.10.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
--
- Jeremiah Mahler
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
2014-12-31 17:45 ` Jeremiah Mahler
@ 2014-12-31 18:15 ` roopa
2014-12-31 18:48 ` Jeremiah Mahler
0 siblings, 1 reply; 7+ messages in thread
From: roopa @ 2014-12-31 18:15 UTC (permalink / raw)
To: Jeremiah Mahler, netdev, shemminger, vyasevic, sfeldma, wkok
On 12/31/14, 9:45 AM, Jeremiah Mahler wrote:
> Roopa,
>
> On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch adds (as suggested by scott feldman),
>> - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
>> vlan list
>> - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
>> BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> include/uapi/linux/if_bridge.h | 4 ++++
>> net/bridge/br_netlink.c | 1 +
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index b03ee8f..fa468aa 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -112,12 +112,14 @@ struct __fdb_entry {
>> * [IFLA_BRIDGE_FLAGS]
>> * [IFLA_BRIDGE_MODE]
>> * [IFLA_BRIDGE_VLAN_INFO]
>> + * [IFLA_BRIDGE_VLAN_INFO_LIST]
>> * }
>> */
>> enum {
>> IFLA_BRIDGE_FLAGS,
>> IFLA_BRIDGE_MODE,
>> IFLA_BRIDGE_VLAN_INFO,
>> + IFLA_BRIDGE_VLAN_INFO_LIST,
>> __IFLA_BRIDGE_MAX,
>> };
>> #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
>> @@ -125,6 +127,8 @@ enum {
>> #define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
>> #define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
>> #define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
>> +#define BRIDGE_VLAN_INFO_RANGE_START (1<<3) /* VLAN is start of vlan range */
>> +#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
>>
> You add these here but you don't use them until the next patch.
> If they were wrong a bisect would point to the next patch.
>
> I would add them in the next patch where you start to use them.
I thought it was ok to declare it first and use them in the next patch.
Only the other way around would be bad.
I have submitted in a similar way before. If needed i will resubmit.
>> struct bridge_vlan_info {
>> __u16 flags;
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 9f5eb55..492ef6a 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
>> [IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
>> [IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
>> .len = sizeof(struct bridge_vlan_info), },
>> + [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
>> };
>>
>> static int br_afspec(struct net_bridge *br,
>> --
>> 1.7.10.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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
2014-12-31 18:15 ` roopa
@ 2014-12-31 18:48 ` Jeremiah Mahler
2014-12-31 21:17 ` roopa
0 siblings, 1 reply; 7+ messages in thread
From: Jeremiah Mahler @ 2014-12-31 18:48 UTC (permalink / raw)
To: roopa; +Cc: netdev, shemminger, vyasevic, sfeldma, wkok
Roopa,
On Wed, Dec 31, 2014 at 10:15:53AM -0800, roopa wrote:
> On 12/31/14, 9:45 AM, Jeremiah Mahler wrote:
> >Roopa,
> >
> >On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
> >>From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>
> >>This patch adds (as suggested by scott feldman),
> >> - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
> >> vlan list
> >> - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
> >> BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
> >>
> >>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>---
> >> include/uapi/linux/if_bridge.h | 4 ++++
> >> net/bridge/br_netlink.c | 1 +
> >> 2 files changed, 5 insertions(+)
> >>
> >>diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> >>index b03ee8f..fa468aa 100644
> >>--- a/include/uapi/linux/if_bridge.h
> >>+++ b/include/uapi/linux/if_bridge.h
> >>@@ -112,12 +112,14 @@ struct __fdb_entry {
> >> * [IFLA_BRIDGE_FLAGS]
> >> * [IFLA_BRIDGE_MODE]
> >> * [IFLA_BRIDGE_VLAN_INFO]
> >>+ * [IFLA_BRIDGE_VLAN_INFO_LIST]
> >> * }
> >> */
> >> enum {
> >> IFLA_BRIDGE_FLAGS,
> >> IFLA_BRIDGE_MODE,
> >> IFLA_BRIDGE_VLAN_INFO,
> >>+ IFLA_BRIDGE_VLAN_INFO_LIST,
> >> __IFLA_BRIDGE_MAX,
> >> };
> >> #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
> >>@@ -125,6 +127,8 @@ enum {
> >> #define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
> >> #define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
> >> #define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
> >>+#define BRIDGE_VLAN_INFO_RANGE_START (1<<3) /* VLAN is start of vlan range */
> >>+#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
> >You add these here but you don't use them until the next patch.
> >If they were wrong a bisect would point to the next patch.
> >
> >I would add them in the next patch where you start to use them.
> I thought it was ok to declare it first and use them in the next patch. Only
> the other way around would be bad.
> I have submitted in a similar way before. If needed i will resubmit.
>
>
Hmm. I cannot see how the other way would be bad but maybe I am missing
something. Hopefully someone else has some insight.
>
>
> >> struct bridge_vlan_info {
> >> __u16 flags;
> >>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> >>index 9f5eb55..492ef6a 100644
> >>--- a/net/bridge/br_netlink.c
> >>+++ b/net/bridge/br_netlink.c
> >>@@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
> >> [IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
> >> [IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
> >> .len = sizeof(struct bridge_vlan_info), },
> >>+ [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
> >> };
> >> static int br_afspec(struct net_bridge *br,
> >>--
> >>1.7.10.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
>
--
- Jeremiah Mahler
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
2014-12-31 18:48 ` Jeremiah Mahler
@ 2014-12-31 21:17 ` roopa
2015-01-01 3:08 ` Jeremiah Mahler
0 siblings, 1 reply; 7+ messages in thread
From: roopa @ 2014-12-31 21:17 UTC (permalink / raw)
To: Jeremiah Mahler, netdev, shemminger, vyasevic, sfeldma, wkok
On 12/31/14, 10:48 AM, Jeremiah Mahler wrote:
> Roopa,
>
> On Wed, Dec 31, 2014 at 10:15:53AM -0800, roopa wrote:
>> On 12/31/14, 9:45 AM, Jeremiah Mahler wrote:
>>> Roopa,
>>>
>>> On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This patch adds (as suggested by scott feldman),
>>>> - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
>>>> vlan list
>>>> - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
>>>> BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
>>>>
>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>> ---
>>>> include/uapi/linux/if_bridge.h | 4 ++++
>>>> net/bridge/br_netlink.c | 1 +
>>>> 2 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>>>> index b03ee8f..fa468aa 100644
>>>> --- a/include/uapi/linux/if_bridge.h
>>>> +++ b/include/uapi/linux/if_bridge.h
>>>> @@ -112,12 +112,14 @@ struct __fdb_entry {
>>>> * [IFLA_BRIDGE_FLAGS]
>>>> * [IFLA_BRIDGE_MODE]
>>>> * [IFLA_BRIDGE_VLAN_INFO]
>>>> + * [IFLA_BRIDGE_VLAN_INFO_LIST]
>>>> * }
>>>> */
>>>> enum {
>>>> IFLA_BRIDGE_FLAGS,
>>>> IFLA_BRIDGE_MODE,
>>>> IFLA_BRIDGE_VLAN_INFO,
>>>> + IFLA_BRIDGE_VLAN_INFO_LIST,
>>>> __IFLA_BRIDGE_MAX,
>>>> };
>>>> #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
>>>> @@ -125,6 +127,8 @@ enum {
>>>> #define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
>>>> #define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
>>>> #define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
>>>> +#define BRIDGE_VLAN_INFO_RANGE_START (1<<3) /* VLAN is start of vlan range */
>>>> +#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
>>> You add these here but you don't use them until the next patch.
>>> If they were wrong a bisect would point to the next patch.
>>>
>>> I would add them in the next patch where you start to use them.
>> I thought it was ok to declare it first and use them in the next patch. Only
>> the other way around would be bad.
>> I have submitted in a similar way before. If needed i will resubmit.
>>
>>
> Hmm. I cannot see how the other way would be bad but maybe I am missing
> something.
sorry, i did not mean what you were saying would be bad. I was just
trying to say that, use first and declare later would be bad (ie if my
patches 1 and 2 were swapped). Otherwise i don't see a problem.
I know that you are saying i should combine the patches 1 and 2 into a
single patch. That is not a problem. If i need to respin again due to
other reasons i will consider merging them as well if that is a concern.
thanks.
> Hopefully someone else has some insight.
>
>>
>>>> struct bridge_vlan_info {
>>>> __u16 flags;
>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>> index 9f5eb55..492ef6a 100644
>>>> --- a/net/bridge/br_netlink.c
>>>> +++ b/net/bridge/br_netlink.c
>>>> @@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
>>>> [IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
>>>> [IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
>>>> .len = sizeof(struct bridge_vlan_info), },
>>>> + [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
>>>> };
>>>> static int br_afspec(struct net_bridge *br,
>>>> --
>>>> 1.7.10.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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
2014-12-31 21:17 ` roopa
@ 2015-01-01 3:08 ` Jeremiah Mahler
2015-01-01 4:25 ` roopa
0 siblings, 1 reply; 7+ messages in thread
From: Jeremiah Mahler @ 2015-01-01 3:08 UTC (permalink / raw)
To: roopa; +Cc: netdev, shemminger, vyasevic, sfeldma, wkok
Roopa,
On Wed, Dec 31, 2014 at 01:17:31PM -0800, roopa wrote:
> On 12/31/14, 10:48 AM, Jeremiah Mahler wrote:
> >Roopa,
> >
> >On Wed, Dec 31, 2014 at 10:15:53AM -0800, roopa wrote:
> >>On 12/31/14, 9:45 AM, Jeremiah Mahler wrote:
> >>>Roopa,
> >>>
> >>>On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
> >>>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>>>
> >>>>This patch adds (as suggested by scott feldman),
> >>>> - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
> >>>> vlan list
> >>>> - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
> >>>> BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
> >>>>
> >>>>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>>>---
> >>>> include/uapi/linux/if_bridge.h | 4 ++++
> >>>> net/bridge/br_netlink.c | 1 +
> >>>> 2 files changed, 5 insertions(+)
> >>>>
> >>>>diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> >>>>index b03ee8f..fa468aa 100644
> >>>>--- a/include/uapi/linux/if_bridge.h
> >>>>+++ b/include/uapi/linux/if_bridge.h
> >>>>@@ -112,12 +112,14 @@ struct __fdb_entry {
> >>>> * [IFLA_BRIDGE_FLAGS]
> >>>> * [IFLA_BRIDGE_MODE]
> >>>> * [IFLA_BRIDGE_VLAN_INFO]
> >>>>+ * [IFLA_BRIDGE_VLAN_INFO_LIST]
> >>>> * }
> >>>> */
> >>>> enum {
> >>>> IFLA_BRIDGE_FLAGS,
> >>>> IFLA_BRIDGE_MODE,
> >>>> IFLA_BRIDGE_VLAN_INFO,
> >>>>+ IFLA_BRIDGE_VLAN_INFO_LIST,
> >>>> __IFLA_BRIDGE_MAX,
> >>>> };
> >>>> #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
> >>>>@@ -125,6 +127,8 @@ enum {
> >>>> #define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
> >>>> #define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
> >>>> #define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
> >>>>+#define BRIDGE_VLAN_INFO_RANGE_START (1<<3) /* VLAN is start of vlan range */
> >>>>+#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
> >>>You add these here but you don't use them until the next patch.
> >>>If they were wrong a bisect would point to the next patch.
> >>>
> >>>I would add them in the next patch where you start to use them.
> >>I thought it was ok to declare it first and use them in the next patch. Only
> >>the other way around would be bad.
> >> I have submitted in a similar way before. If needed i will resubmit.
> >>
> >>
> >Hmm. I cannot see how the other way would be bad but maybe I am missing
> >something.
> sorry, i did not mean what you were saying would be bad. I was just trying
> to say that, use first and declare later would be bad (ie if my patches 1
> and 2 were swapped). Otherwise i don't see a problem.
>
Now I understand. Yes, swapping the patches would be bad.
> I know that you are saying i should combine the patches 1 and 2 into a
> single patch. That is not a problem. If i need to respin again due to other
> reasons i will consider merging them as well if that is a concern.
>
Er, well not quite. I don't think both patches should be combined in to
one. I would only move those two #defines that I pointed out in the
first patch in to the second patch.
I hope that makes a little more sense :)
> thanks.
>
> > Hopefully someone else has some insight.
> >
> >>
> >>>> struct bridge_vlan_info {
> >>>> __u16 flags;
> >>>>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> >>>>index 9f5eb55..492ef6a 100644
> >>>>--- a/net/bridge/br_netlink.c
> >>>>+++ b/net/bridge/br_netlink.c
> >>>>@@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
> >>>> [IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
> >>>> [IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
> >>>> .len = sizeof(struct bridge_vlan_info), },
> >>>>+ [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
> >>>> };
> >>>> static int br_afspec(struct net_bridge *br,
> >>>>--
> >>>>1.7.10.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
>
--
- Jeremiah Mahler
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
2015-01-01 3:08 ` Jeremiah Mahler
@ 2015-01-01 4:25 ` roopa
0 siblings, 0 replies; 7+ messages in thread
From: roopa @ 2015-01-01 4:25 UTC (permalink / raw)
To: Jeremiah Mahler, netdev, shemminger, vyasevic, sfeldma, wkok
On 12/31/14, 7:08 PM, Jeremiah Mahler wrote:
> Roopa,
>
> On Wed, Dec 31, 2014 at 01:17:31PM -0800, roopa wrote:
>> On 12/31/14, 10:48 AM, Jeremiah Mahler wrote:
>>> Roopa,
>>>
>>> On Wed, Dec 31, 2014 at 10:15:53AM -0800, roopa wrote:
>>>> On 12/31/14, 9:45 AM, Jeremiah Mahler wrote:
>>>>> Roopa,
>>>>>
>>>>> On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
>>>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>>
>>>>>> This patch adds (as suggested by scott feldman),
>>>>>> - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
>>>>>> vlan list
>>>>>> - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
>>>>>> BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
>>>>>>
>>>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>> ---
>>>>>> include/uapi/linux/if_bridge.h | 4 ++++
>>>>>> net/bridge/br_netlink.c | 1 +
>>>>>> 2 files changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>>>>>> index b03ee8f..fa468aa 100644
>>>>>> --- a/include/uapi/linux/if_bridge.h
>>>>>> +++ b/include/uapi/linux/if_bridge.h
>>>>>> @@ -112,12 +112,14 @@ struct __fdb_entry {
>>>>>> * [IFLA_BRIDGE_FLAGS]
>>>>>> * [IFLA_BRIDGE_MODE]
>>>>>> * [IFLA_BRIDGE_VLAN_INFO]
>>>>>> + * [IFLA_BRIDGE_VLAN_INFO_LIST]
>>>>>> * }
>>>>>> */
>>>>>> enum {
>>>>>> IFLA_BRIDGE_FLAGS,
>>>>>> IFLA_BRIDGE_MODE,
>>>>>> IFLA_BRIDGE_VLAN_INFO,
>>>>>> + IFLA_BRIDGE_VLAN_INFO_LIST,
>>>>>> __IFLA_BRIDGE_MAX,
>>>>>> };
>>>>>> #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
>>>>>> @@ -125,6 +127,8 @@ enum {
>>>>>> #define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
>>>>>> #define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
>>>>>> #define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */
>>>>>> +#define BRIDGE_VLAN_INFO_RANGE_START (1<<3) /* VLAN is start of vlan range */
>>>>>> +#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
>>>>> You add these here but you don't use them until the next patch.
>>>>> If they were wrong a bisect would point to the next patch.
>>>>>
>>>>> I would add them in the next patch where you start to use them.
>>>> I thought it was ok to declare it first and use them in the next patch. Only
>>>> the other way around would be bad.
>>>> I have submitted in a similar way before. If needed i will resubmit.
>>>>
>>>>
>>> Hmm. I cannot see how the other way would be bad but maybe I am missing
>>> something.
>> sorry, i did not mean what you were saying would be bad. I was just trying
>> to say that, use first and declare later would be bad (ie if my patches 1
>> and 2 were swapped). Otherwise i don't see a problem.
>>
> Now I understand. Yes, swapping the patches would be bad.
>
>> I know that you are saying i should combine the patches 1 and 2 into a
>> single patch. That is not a problem. If i need to respin again due to other
>> reasons i will consider merging them as well if that is a concern.
>>
> Er, well not quite. I don't think both patches should be combined in to
> one. I would only move those two #defines that I pointed out in the
> first patch in to the second patch.
>
> I hope that makes a little more sense :)
okay :).
thanks,
Roopa
>
>> thanks.
>>
>>> Hopefully someone else has some insight.
>>>
>>>>>> struct bridge_vlan_info {
>>>>>> __u16 flags;
>>>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>>>> index 9f5eb55..492ef6a 100644
>>>>>> --- a/net/bridge/br_netlink.c
>>>>>> +++ b/net/bridge/br_netlink.c
>>>>>> @@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
>>>>>> [IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
>>>>>> [IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
>>>>>> .len = sizeof(struct bridge_vlan_info), },
>>>>>> + [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
>>>>>> };
>>>>>> static int br_afspec(struct net_bridge *br,
>>>>>> --
>>>>>> 1.7.10.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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-01 4:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-31 16:48 [PATCH net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges roopa
2014-12-31 17:45 ` Jeremiah Mahler
2014-12-31 18:15 ` roopa
2014-12-31 18:48 ` Jeremiah Mahler
2014-12-31 21:17 ` roopa
2015-01-01 3:08 ` Jeremiah Mahler
2015-01-01 4:25 ` roopa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).