netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi
@ 2019-12-11 22:30 Andy Roulin
  2019-12-11 22:47 ` Jay Vosburgh
  2019-12-14 21:18 ` Jakub Kicinski
  0 siblings, 2 replies; 6+ messages in thread
From: Andy Roulin @ 2019-12-11 22:30 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, nikolay, roopa, j.vosburgh, vfalico, andy, stephen

The bond slave actor/partner operating state is exported as
bitfield to userspace, which lacks a way to interpret it, e.g.,
iproute2 only prints the state as a number:

ad_actor_oper_port_state 15

For userspace to interpret the bitfield, the bitfield definitions
should be part of the uapi. The bitfield itself is defined in the
802.3ad standard.

This commit moves the 802.3ad bitfield definitions to uapi.

Related iproute2 patches, soon to be posted upstream, use the new uapi
headers to pretty-print bond slave state, e.g., with ip -d link show

ad_actor_oper_port_state_str <active,short_timeout,aggregating,in_sync>

Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 drivers/net/bonding/bond_3ad.c  | 10 ----------
 include/uapi/linux/if_bonding.h | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index e3b25f310936..34bfe99641a3 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -31,16 +31,6 @@
 #define AD_CHURN_DETECTION_TIME    60
 #define AD_AGGREGATE_WAIT_TIME     2
 
-/* Port state definitions (43.4.2.2 in the 802.3ad standard) */
-#define AD_STATE_LACP_ACTIVITY   0x1
-#define AD_STATE_LACP_TIMEOUT    0x2
-#define AD_STATE_AGGREGATION     0x4
-#define AD_STATE_SYNCHRONIZATION 0x8
-#define AD_STATE_COLLECTING      0x10
-#define AD_STATE_DISTRIBUTING    0x20
-#define AD_STATE_DEFAULTED       0x40
-#define AD_STATE_EXPIRED         0x80
-
 /* Port Variables definitions used by the State Machines (43.4.7 in the
  * 802.3ad standard)
  */
diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h
index 790585f0e61b..6829213a54c5 100644
--- a/include/uapi/linux/if_bonding.h
+++ b/include/uapi/linux/if_bonding.h
@@ -95,6 +95,16 @@
 #define BOND_XMIT_POLICY_ENCAP23	3 /* encapsulated layer 2+3 */
 #define BOND_XMIT_POLICY_ENCAP34	4 /* encapsulated layer 3+4 */
 
+/* 802.3ad port state definitions (43.4.2.2 in the 802.3ad standard) */
+#define AD_STATE_LACP_ACTIVITY   0x1
+#define AD_STATE_LACP_TIMEOUT    0x2
+#define AD_STATE_AGGREGATION     0x4
+#define AD_STATE_SYNCHRONIZATION 0x8
+#define AD_STATE_COLLECTING      0x10
+#define AD_STATE_DISTRIBUTING    0x20
+#define AD_STATE_DEFAULTED       0x40
+#define AD_STATE_EXPIRED         0x80
+
 typedef struct ifbond {
 	__s32 bond_mode;
 	__s32 num_slaves;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi
  2019-12-11 22:30 [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi Andy Roulin
@ 2019-12-11 22:47 ` Jay Vosburgh
  2019-12-14 21:18 ` Jakub Kicinski
  1 sibling, 0 replies; 6+ messages in thread
From: Jay Vosburgh @ 2019-12-11 22:47 UTC (permalink / raw)
  To: Andy Roulin; +Cc: netdev, dsahern, nikolay, roopa, vfalico, andy, stephen

Andy Roulin <aroulin@cumulusnetworks.com> wrote:

>The bond slave actor/partner operating state is exported as
>bitfield to userspace, which lacks a way to interpret it, e.g.,
>iproute2 only prints the state as a number:
>
>ad_actor_oper_port_state 15
>
>For userspace to interpret the bitfield, the bitfield definitions
>should be part of the uapi. The bitfield itself is defined in the
>802.3ad standard.
>
>This commit moves the 802.3ad bitfield definitions to uapi.
>
>Related iproute2 patches, soon to be posted upstream, use the new uapi
>headers to pretty-print bond slave state, e.g., with ip -d link show
>
>ad_actor_oper_port_state_str <active,short_timeout,aggregating,in_sync>
>
>Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
>Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>


>---
> drivers/net/bonding/bond_3ad.c  | 10 ----------
> include/uapi/linux/if_bonding.h | 10 ++++++++++
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index e3b25f310936..34bfe99641a3 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -31,16 +31,6 @@
> #define AD_CHURN_DETECTION_TIME    60
> #define AD_AGGREGATE_WAIT_TIME     2
> 
>-/* Port state definitions (43.4.2.2 in the 802.3ad standard) */
>-#define AD_STATE_LACP_ACTIVITY   0x1
>-#define AD_STATE_LACP_TIMEOUT    0x2
>-#define AD_STATE_AGGREGATION     0x4
>-#define AD_STATE_SYNCHRONIZATION 0x8
>-#define AD_STATE_COLLECTING      0x10
>-#define AD_STATE_DISTRIBUTING    0x20
>-#define AD_STATE_DEFAULTED       0x40
>-#define AD_STATE_EXPIRED         0x80
>-
> /* Port Variables definitions used by the State Machines (43.4.7 in the
>  * 802.3ad standard)
>  */
>diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h
>index 790585f0e61b..6829213a54c5 100644
>--- a/include/uapi/linux/if_bonding.h
>+++ b/include/uapi/linux/if_bonding.h
>@@ -95,6 +95,16 @@
> #define BOND_XMIT_POLICY_ENCAP23	3 /* encapsulated layer 2+3 */
> #define BOND_XMIT_POLICY_ENCAP34	4 /* encapsulated layer 3+4 */
> 
>+/* 802.3ad port state definitions (43.4.2.2 in the 802.3ad standard) */
>+#define AD_STATE_LACP_ACTIVITY   0x1
>+#define AD_STATE_LACP_TIMEOUT    0x2
>+#define AD_STATE_AGGREGATION     0x4
>+#define AD_STATE_SYNCHRONIZATION 0x8
>+#define AD_STATE_COLLECTING      0x10
>+#define AD_STATE_DISTRIBUTING    0x20
>+#define AD_STATE_DEFAULTED       0x40
>+#define AD_STATE_EXPIRED         0x80
>+
> typedef struct ifbond {
> 	__s32 bond_mode;
> 	__s32 num_slaves;
>-- 
>2.20.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi
  2019-12-11 22:30 [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi Andy Roulin
  2019-12-11 22:47 ` Jay Vosburgh
@ 2019-12-14 21:18 ` Jakub Kicinski
  2019-12-16  3:10   ` David Ahern
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2019-12-14 21:18 UTC (permalink / raw)
  To: Andy Roulin
  Cc: netdev, dsahern, nikolay, roopa, j.vosburgh, vfalico, andy,
	stephen

On Wed, 11 Dec 2019 14:30:58 -0800, Andy Roulin wrote:
> The bond slave actor/partner operating state is exported as
> bitfield to userspace, which lacks a way to interpret it, e.g.,
> iproute2 only prints the state as a number:
> 
> ad_actor_oper_port_state 15
> 
> For userspace to interpret the bitfield, the bitfield definitions
> should be part of the uapi. The bitfield itself is defined in the
> 802.3ad standard.
> 
> This commit moves the 802.3ad bitfield definitions to uapi.
> 
> Related iproute2 patches, soon to be posted upstream, use the new uapi
> headers to pretty-print bond slave state, e.g., with ip -d link show
> 
> ad_actor_oper_port_state_str <active,short_timeout,aggregating,in_sync>
> 
> Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>

Applied, I wonder if it wouldn't be better to rename those
s/AD_/BOND_3AD_/ like the prefix the stats have. 
But I guess it's unlikely user space has those exact defines 
set to a different value so can't cause a clash..

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi
  2019-12-14 21:18 ` Jakub Kicinski
@ 2019-12-16  3:10   ` David Ahern
  2019-12-16  3:13     ` Andy Gospodarek
  0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2019-12-16  3:10 UTC (permalink / raw)
  To: Jakub Kicinski, Andy Roulin
  Cc: netdev, nikolay, roopa, j.vosburgh, vfalico, andy, stephen

On 12/14/19 2:18 PM, Jakub Kicinski wrote:
> On Wed, 11 Dec 2019 14:30:58 -0800, Andy Roulin wrote:
>> The bond slave actor/partner operating state is exported as
>> bitfield to userspace, which lacks a way to interpret it, e.g.,
>> iproute2 only prints the state as a number:
>>
>> ad_actor_oper_port_state 15
>>
>> For userspace to interpret the bitfield, the bitfield definitions
>> should be part of the uapi. The bitfield itself is defined in the
>> 802.3ad standard.
>>
>> This commit moves the 802.3ad bitfield definitions to uapi.
>>
>> Related iproute2 patches, soon to be posted upstream, use the new uapi
>> headers to pretty-print bond slave state, e.g., with ip -d link show
>>
>> ad_actor_oper_port_state_str <active,short_timeout,aggregating,in_sync>
>>
>> Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
>> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> Applied, I wonder if it wouldn't be better to rename those
> s/AD_/BOND_3AD_/ like the prefix the stats have. 
> But I guess it's unlikely user space has those exact defines 
> set to a different value so can't cause a clash..
> 

I think that would be a better namespace now that it is in the UAPI.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi
  2019-12-16  3:10   ` David Ahern
@ 2019-12-16  3:13     ` Andy Gospodarek
  2019-12-16  3:52       ` Jay Vosburgh
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Gospodarek @ 2019-12-16  3:13 UTC (permalink / raw)
  To: David Ahern
  Cc: Jakub Kicinski, Andy Roulin, netdev, nikolay, roopa, j.vosburgh,
	vfalico, stephen

On Sun, Dec 15, 2019 at 08:10:15PM -0700, David Ahern wrote:
> On 12/14/19 2:18 PM, Jakub Kicinski wrote:
> > On Wed, 11 Dec 2019 14:30:58 -0800, Andy Roulin wrote:
> >> The bond slave actor/partner operating state is exported as
> >> bitfield to userspace, which lacks a way to interpret it, e.g.,
> >> iproute2 only prints the state as a number:
> >>
> >> ad_actor_oper_port_state 15
> >>
> >> For userspace to interpret the bitfield, the bitfield definitions
> >> should be part of the uapi. The bitfield itself is defined in the
> >> 802.3ad standard.
> >>
> >> This commit moves the 802.3ad bitfield definitions to uapi.
> >>
> >> Related iproute2 patches, soon to be posted upstream, use the new uapi
> >> headers to pretty-print bond slave state, e.g., with ip -d link show
> >>
> >> ad_actor_oper_port_state_str <active,short_timeout,aggregating,in_sync>
> >>
> >> Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
> >> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> > 
> > Applied, I wonder if it wouldn't be better to rename those
> > s/AD_/BOND_3AD_/ like the prefix the stats have. 
> > But I guess it's unlikely user space has those exact defines 
> > set to a different value so can't cause a clash..
> > 
> 
> I think that would be a better namespace now that it is in the UAPI.

I agree that it would be nuch nicer.  I never really liked the 'AD'
usage as an abbreviation for 802.3ad.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi
  2019-12-16  3:13     ` Andy Gospodarek
@ 2019-12-16  3:52       ` Jay Vosburgh
  0 siblings, 0 replies; 6+ messages in thread
From: Jay Vosburgh @ 2019-12-16  3:52 UTC (permalink / raw)
  To: Andy Gospodarek
  Cc: David Ahern, Jakub Kicinski, Andy Roulin, netdev, nikolay, roopa,
	vfalico, stephen

Andy Gospodarek <andy@greyhouse.net> wrote:

>On Sun, Dec 15, 2019 at 08:10:15PM -0700, David Ahern wrote:
>> On 12/14/19 2:18 PM, Jakub Kicinski wrote:
>> > On Wed, 11 Dec 2019 14:30:58 -0800, Andy Roulin wrote:
>> >> The bond slave actor/partner operating state is exported as
>> >> bitfield to userspace, which lacks a way to interpret it, e.g.,
>> >> iproute2 only prints the state as a number:
>> >>
>> >> ad_actor_oper_port_state 15
>> >>
>> >> For userspace to interpret the bitfield, the bitfield definitions
>> >> should be part of the uapi. The bitfield itself is defined in the
>> >> 802.3ad standard.
>> >>
>> >> This commit moves the 802.3ad bitfield definitions to uapi.
>> >>
>> >> Related iproute2 patches, soon to be posted upstream, use the new uapi
>> >> headers to pretty-print bond slave state, e.g., with ip -d link show
>> >>
>> >> ad_actor_oper_port_state_str <active,short_timeout,aggregating,in_sync>
>> >>
>> >> Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
>> >> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> > 
>> > Applied, I wonder if it wouldn't be better to rename those
>> > s/AD_/BOND_3AD_/ like the prefix the stats have. 
>> > But I guess it's unlikely user space has those exact defines 
>> > set to a different value so can't cause a clash..
>> > 
>> 
>> I think that would be a better namespace now that it is in the UAPI.
>
>I agree that it would be nuch nicer.  I never really liked the 'AD'
>usage as an abbreviation for 802.3ad.

	Agreed, and, of course, the LACP standard moved to 802.1AX about
ten years ago; perhaps replace "AD" with "LACP" in the UAPI?

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-16  3:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-11 22:30 [PATCH net-next v2] bonding: move 802.3ad port state flags to uapi Andy Roulin
2019-12-11 22:47 ` Jay Vosburgh
2019-12-14 21:18 ` Jakub Kicinski
2019-12-16  3:10   ` David Ahern
2019-12-16  3:13     ` Andy Gospodarek
2019-12-16  3:52       ` Jay Vosburgh

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).