* [patch 01/10] 802.3ad: make ntt bool
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 19:18 ` David Miller
2008-12-23 22:01 ` [patch 02/10] 802.3ad: turn ports is_enabled into a bool Holger Eitzenberger
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-ntt-bool.diff --]
[-- Type: text/plain, Size: 3772 bytes --]
Turn Need-To-Transmit port variable into a bool. There is no functional
change.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -215,7 +215,7 @@ typedef struct port {
struct mac_addr actor_system; // This parameter is added here although it is not specified in the standard, just for simplification
u16 actor_system_priority; // This parameter is added here although it is not specified in the standard, just for simplification
u16 actor_port_aggregator_identifier;
- u16 ntt; // BOOLEAN
+ bool ntt;
u16 actor_admin_port_key;
u16 actor_oper_port_key;
u8 actor_admin_port_state;
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -629,8 +629,8 @@ static void __update_ntt(struct lacpdu *
((lacpdu->partner_state & AD_STATE_SYNCHRONIZATION) != (port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION)) ||
((lacpdu->partner_state & AD_STATE_AGGREGATION) != (port->actor_oper_port_state & AD_STATE_AGGREGATION))
) {
- // set ntt to be TRUE
- port->ntt = 1;
+
+ port->ntt = true;
}
}
}
@@ -987,7 +987,7 @@ static void ad_mux_machine(struct port *
ad_disable_collecting_distributing(port);
port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
- port->ntt = 1;
+ port->ntt = true;
break;
case AD_MUX_WAITING:
port->sm_mux_timer_counter = __ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
@@ -998,13 +998,13 @@ static void ad_mux_machine(struct port *
port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
ad_disable_collecting_distributing(port);
- port->ntt = 1;
+ port->ntt = true;
break;
case AD_MUX_COLLECTING_DISTRIBUTING:
port->actor_oper_port_state |= AD_STATE_COLLECTING;
port->actor_oper_port_state |= AD_STATE_DISTRIBUTING;
ad_enable_collecting_distributing(port);
- port->ntt = 1;
+ port->ntt = true;
break;
default: //to silence the compiler
break;
@@ -1163,11 +1163,13 @@ static void ad_tx_machine(struct port *p
// check if there is something to send
if (port->ntt && (port->sm_vars & AD_PORT_LACP_ENABLED)) {
__update_lacpdu_from_port(port);
- // send the lacpdu
+
if (ad_lacpdu_send(port) >= 0) {
pr_debug("Sent LACPDU on port %d\n", port->actor_port_number);
- // mark ntt as false, so it will not be sent again until demanded
- port->ntt = 0;
+
+ /* mark ntt as false, so it will not be sent again until
+ demanded */
+ port->ntt = false;
}
}
// restart tx timer(to verify that we will not exceed AD_MAX_TX_IN_SECOND
@@ -1250,7 +1252,7 @@ static void ad_periodic_machine(struct p
port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_SLOW_PERIODIC_TIME))-1; // decrement 1 tick we lost in the PERIODIC_TX cycle
break;
case AD_PERIODIC_TX:
- port->ntt = 1;
+ port->ntt = true;
break;
default: //to silence the compiler
break;
@@ -1664,7 +1666,7 @@ static void ad_initialize_port(struct po
port->actor_system = null_mac_addr;
port->actor_system_priority = 0xffff;
port->actor_port_aggregator_identifier = 0;
- port->ntt = 0;
+ port->ntt = false;
port->actor_admin_port_key = 1;
port->actor_oper_port_key = 1;
port->actor_admin_port_state = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 02/10] 802.3ad: turn ports is_enabled into a bool
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
2008-12-23 22:01 ` [patch 01/10] 802.3ad: make ntt bool Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 21:27 ` David Miller
2008-12-23 22:01 ` [patch 03/10] 802.3ad: turn ports is_individual " Holger Eitzenberger
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-is_enabled-bool.diff --]
[-- Type: text/plain, Size: 2122 bytes --]
Turn ports is_enabled into a bool. There is no functional change.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -224,7 +224,8 @@ typedef struct port {
struct port_params partner_admin;
struct port_params partner_oper;
- u16 is_enabled; // BOOLEAN
+ bool is_enabled;
+
// ****** PRIVATE PARAMETERS ******
u16 sm_vars; // all state machines variables for this port
rx_states_t sm_rx_state; // state machine rx state
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -1679,7 +1679,7 @@ static void ad_initialize_port(struct po
memcpy(&port->partner_admin, &tmpl, sizeof(tmpl));
memcpy(&port->partner_oper, &tmpl, sizeof(tmpl));
- port->is_enabled = 1;
+ port->is_enabled = true;
// ****** private parameters ******
port->sm_vars = 0x3;
port->sm_rx_state = 0;
@@ -2308,14 +2308,14 @@ void bond_3ad_handle_link_change(struct
// on link down we are zeroing duplex and speed since some of the adaptors(ce1000.lan) report full duplex/speed instead of N/A(duplex) / 0(speed)
// on link up we are forcing recheck on the duplex and speed since some of he adaptors(ce1000.lan) report
if (link == BOND_LINK_UP) {
- port->is_enabled = 1;
+ port->is_enabled = true;
port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
port->actor_oper_port_key=port->actor_admin_port_key |= __get_duplex(port);
port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS;
port->actor_oper_port_key=port->actor_admin_port_key |= (__get_link_speed(port) << 1);
} else {
/* link has failed */
- port->is_enabled = 0;
+ port->is_enabled = false;
port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
port->actor_oper_port_key= (port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS);
}
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 03/10] 802.3ad: turn ports is_individual into a bool
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
2008-12-23 22:01 ` [patch 01/10] 802.3ad: make ntt bool Holger Eitzenberger
2008-12-23 22:01 ` [patch 02/10] 802.3ad: turn ports is_enabled into a bool Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 21:27 ` David Miller
2008-12-23 22:01 ` [patch 04/10] 802.3ad: remove typedef around ad_system Holger Eitzenberger
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev, Holger Eitzenberger
[-- Attachment #1: bonding-3ad-is_individual-bool.diff --]
[-- Type: text/plain, Size: 1732 bytes --]
Turn ports is_individual into a bool. There is no functional change.
Signed-off-by: Holger Eitzenberger <heitzenberger@astaro.com>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -1359,9 +1359,9 @@ static void ad_port_selection_logic(stru
// update the new aggregator's parameters
// if port was responsed from the end-user
if (port->actor_oper_port_key & AD_DUPLEX_KEY_BITS) {// if port is full duplex
- port->aggregator->is_individual = 0;
+ port->aggregator->is_individual = false;
} else {
- port->aggregator->is_individual = 1;
+ port->aggregator->is_individual = true;
}
port->aggregator->actor_admin_aggregator_key = port->actor_admin_port_key;
@@ -1613,7 +1613,7 @@ static void ad_agg_selection_logic(struc
static void ad_clear_agg(struct aggregator *aggregator)
{
if (aggregator) {
- aggregator->is_individual = 0;
+ aggregator->is_individual = false;
aggregator->actor_admin_aggregator_key = 0;
aggregator->actor_oper_aggregator_key = 0;
aggregator->partner_system = null_mac_addr;
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -184,7 +184,7 @@ struct port;
typedef struct aggregator {
struct mac_addr aggregator_mac_address;
u16 aggregator_identifier;
- u16 is_individual; // BOOLEAN
+ bool is_individual;
u16 actor_admin_aggregator_key;
u16 actor_oper_aggregator_key;
struct mac_addr partner_system;
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 04/10] 802.3ad: remove typedef around ad_system
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
` (2 preceding siblings ...)
2008-12-23 22:01 ` [patch 03/10] 802.3ad: turn ports is_individual " Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 21:28 ` David Miller
2008-12-23 22:01 ` [patch 05/10] 802.3ad: initialize ports LACPDU from const initializer Holger Eitzenberger
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev, Holger Eitzenberger
[-- Attachment #1: bonding-3ad-remove-unused-typedef.diff --]
[-- Type: text/plain, Size: 1081 bytes --]
As typedefs are considered a bad thing most of the time remove the
typedef around ad_system.
Signed-off-by: Holger Eitzenberger <heitzenberger@astaro.com>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -244,10 +244,10 @@ typedef struct port {
} port_t;
// system structure
-typedef struct ad_system {
+struct ad_system {
u16 sys_priority;
struct mac_addr sys_mac_addr;
-} ad_system_t;
+};
#ifdef __ia64__
#pragma pack()
@@ -258,7 +258,7 @@ typedef struct ad_system {
#define SLAVE_AD_INFO(slave) ((slave)->ad_info)
struct ad_bond_info {
- ad_system_t system; // 802.3ad system structure
+ struct ad_system system; /* 802.3ad system structure */
u32 agg_select_timer; // Timer to select aggregator after all adapter's hand shakes
u32 agg_select_mode; // Mode of selection of active aggregator(bandwidth/count)
int lacp_fast; /* whether fast periodic tx should be
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 05/10] 802.3ad: initialize ports LACPDU from const initializer
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
` (3 preceding siblings ...)
2008-12-23 22:01 ` [patch 04/10] 802.3ad: remove typedef around ad_system Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 21:28 ` David Miller
2008-12-23 22:01 ` [patch 06/10] 802.3ad: generalize out mac address initializer Holger Eitzenberger
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-initialize_lacpdu-initializer.diff --]
[-- Type: text/plain, Size: 3730 bytes --]
Save some text by initializing ports LACPDU from const initializer,
then get rid of ad_initialize_lacpdu().
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -107,7 +107,6 @@ static void ad_agg_selection_logic(struc
static void ad_clear_agg(struct aggregator *aggregator);
static void ad_initialize_agg(struct aggregator *aggregator);
static void ad_initialize_port(struct port *port, int lacp_fast);
-static void ad_initialize_lacpdu(struct lacpdu *Lacpdu);
static void ad_enable_collecting_distributing(struct port *port);
static void ad_disable_collecting_distributing(struct port *port);
static void ad_marker_info_received(struct bond_marker *marker_info, struct port *port);
@@ -1659,6 +1658,17 @@ static void ad_initialize_port(struct po
.port_priority = 0xff,
.port_state = 1,
};
+ static const struct lacpdu lacpdu = {
+ .subtype = 0x01,
+ .version_number = 0x01,
+ .tlv_type_actor_info = 0x01,
+ .actor_information_length = 0x14,
+ .tlv_type_partner_info = 0x02,
+ .partner_information_length = 0x14,
+ .tlv_type_collector_info = 0x03,
+ .collector_information_length = 0x10,
+ .collector_max_delay = htons(AD_COLLECTOR_MAX_DELAY),
+ };
if (port) {
port->actor_port_number = 1;
@@ -1695,7 +1705,7 @@ static void ad_initialize_port(struct po
port->next_port_in_aggregator = NULL;
port->transaction_id = 0;
- ad_initialize_lacpdu(&(port->lacpdu));
+ memcpy(&port->lacpdu, &lacpdu, sizeof(lacpdu));
}
}
@@ -1804,53 +1814,6 @@ static void ad_marker_response_received(
// DO NOTHING, SINCE WE DECIDED NOT TO IMPLEMENT THIS FEATURE FOR NOW
}
-/**
- * ad_initialize_lacpdu - initialize a given lacpdu structure
- * @lacpdu: lacpdu structure to initialize
- *
- */
-static void ad_initialize_lacpdu(struct lacpdu *lacpdu)
-{
- u16 index;
-
- // initialize lacpdu data
- lacpdu->subtype = 0x01;
- lacpdu->version_number = 0x01;
- lacpdu->tlv_type_actor_info = 0x01;
- lacpdu->actor_information_length = 0x14;
- // lacpdu->actor_system_priority updated on send
- // lacpdu->actor_system updated on send
- // lacpdu->actor_key updated on send
- // lacpdu->actor_port_priority updated on send
- // lacpdu->actor_port updated on send
- // lacpdu->actor_state updated on send
- lacpdu->tlv_type_partner_info = 0x02;
- lacpdu->partner_information_length = 0x14;
- for (index=0; index<=2; index++) {
- lacpdu->reserved_3_1[index]=0;
- }
- // lacpdu->partner_system_priority updated on send
- // lacpdu->partner_system updated on send
- // lacpdu->partner_key updated on send
- // lacpdu->partner_port_priority updated on send
- // lacpdu->partner_port updated on send
- // lacpdu->partner_state updated on send
- for (index=0; index<=2; index++) {
- lacpdu->reserved_3_2[index]=0;
- }
- lacpdu->tlv_type_collector_info = 0x03;
- lacpdu->collector_information_length= 0x10;
- lacpdu->collector_max_delay = htons(AD_COLLECTOR_MAX_DELAY);
- for (index=0; index<=11; index++) {
- lacpdu->reserved_12[index]=0;
- }
- lacpdu->tlv_type_terminator = 0x00;
- lacpdu->terminator_length = 0;
- for (index=0; index<=49; index++) {
- lacpdu->reserved_50[index]=0;
- }
-}
-
//////////////////////////////////////////////////////////////////////////////////////
// ================= AD exported functions to the main bonding code ==================
//////////////////////////////////////////////////////////////////////////////////////
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 06/10] 802.3ad: generalize out mac address initializer
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
` (4 preceding siblings ...)
2008-12-23 22:01 ` [patch 05/10] 802.3ad: initialize ports LACPDU from const initializer Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 21:40 ` David Miller
2008-12-23 22:01 ` [patch 07/10] 802.3ad: use standard ethhdr instead of ad_header Holger Eitzenberger
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-mcast-addr-initializer.diff --]
[-- Type: text/plain, Size: 3692 bytes --]
Generalize out mac address initializer for the LACPDU multicast
address and use in two places. Remove the now unused
AD_MULTICAST_LACPDU_ADDR.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -95,6 +95,8 @@ static struct mac_addr null_mac_addr = {
static u16 ad_ticks_per_sec;
static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000;
+static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
+
// ================= main 802.3ad protocol functions ==================
static int ad_lacpdu_send(struct port *port);
static int ad_marker_send(struct port *port, struct bond_marker *marker);
@@ -824,7 +826,6 @@ static int ad_lacpdu_send(struct port *p
struct sk_buff *skb;
struct lacpdu_header *lacpdu_header;
int length = sizeof(struct lacpdu_header);
- struct mac_addr lacpdu_multicast_address = AD_MULTICAST_LACPDU_ADDR;
skb = dev_alloc_skb(length);
if (!skb) {
@@ -839,10 +840,12 @@ static int ad_lacpdu_send(struct port *p
lacpdu_header = (struct lacpdu_header *)skb_put(skb, length);
- lacpdu_header->ad_header.destination_address = lacpdu_multicast_address;
- /* Note: source addres is set to be the member's PERMANENT address, because we use it
- to identify loopback lacpdus in receive. */
- lacpdu_header->ad_header.source_address = *((struct mac_addr *)(slave->perm_hwaddr));
+ memcpy(lacpdu_header->ad_header.destination_address.mac_addr_value,
+ lacpdu_mcast_addr, ETH_ALEN);
+ /* Note: source addres is set to be the member's PERMANENT address,
+ because we use it to identify loopback lacpdus in receive. */
+ memcpy(lacpdu_header->ad_header.source_address.mac_addr_value,
+ slave->perm_hwaddr, ETH_ALEN);
lacpdu_header->ad_header.length_type = PKT_TYPE_LACPDU;
lacpdu_header->lacpdu = port->lacpdu; // struct copy
@@ -866,7 +869,6 @@ static int ad_marker_send(struct port *p
struct sk_buff *skb;
struct bond_marker_header *marker_header;
int length = sizeof(struct bond_marker_header);
- struct mac_addr lacpdu_multicast_address = AD_MULTICAST_LACPDU_ADDR;
skb = dev_alloc_skb(length + 16);
if (!skb) {
@@ -882,10 +884,12 @@ static int ad_marker_send(struct port *p
marker_header = (struct bond_marker_header *)skb_put(skb, length);
- marker_header->ad_header.destination_address = lacpdu_multicast_address;
- /* Note: source addres is set to be the member's PERMANENT address, because we use it
- to identify loopback MARKERs in receive. */
- marker_header->ad_header.source_address = *((struct mac_addr *)(slave->perm_hwaddr));
+ memcpy(marker_header->ad_header.destination_address.mac_addr_value,
+ lacpdu_mcast_addr, ETH_ALEN);
+ /* Note: source addres is set to be the member's PERMANENT address,
+ because we use it to identify loopback MARKERs in receive. */
+ memcpy(marker_header->ad_header.source_address.mac_addr_value,
+ slave->perm_hwaddr, ETH_ALEN);
marker_header->ad_header.length_type = PKT_TYPE_LACPDU;
marker_header->marker = *marker; // struct copy
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -33,7 +33,6 @@
#define AD_TIMER_INTERVAL 100 /*msec*/
#define MULTICAST_LACPDU_ADDR {0x01, 0x80, 0xC2, 0x00, 0x00, 0x02}
-#define AD_MULTICAST_LACPDU_ADDR {MULTICAST_LACPDU_ADDR}
#define AD_LACP_SLOW 0
#define AD_LACP_FAST 1
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 07/10] 802.3ad: use standard ethhdr instead of ad_header
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
` (5 preceding siblings ...)
2008-12-23 22:01 ` [patch 06/10] 802.3ad: generalize out mac address initializer Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 21:41 ` David Miller
2008-12-23 22:01 ` [patch 08/10] 802.3ad: remove public lacpdu_header Holger Eitzenberger
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-ad_header-is-ethhdr.diff --]
[-- Type: text/plain, Size: 3110 bytes --]
802.3ad has its own ethhdr-like structure in the form of an ad_header,
which is at the start of both the LACPDU and marker PDU. Both are
the same from the struct values, both are packed as well.
It's therefore perfectly fine to replace the ad_header by the ethhdr
and to remove its definition.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -105,12 +105,6 @@ typedef enum {
#pragma pack(1)
-typedef struct ad_header {
- struct mac_addr destination_address;
- struct mac_addr source_address;
- __be16 length_type;
-} ad_header_t;
-
// Link Aggregation Control Protocol(LACP) data unit structure(43.4.2.2 in the 802.3ad standard)
typedef struct lacpdu {
u8 subtype; // = LACP(= 0x01)
@@ -143,7 +137,7 @@ typedef struct lacpdu {
} lacpdu_t;
typedef struct lacpdu_header {
- struct ad_header ad_header;
+ struct ethhdr hdr;
struct lacpdu lacpdu;
} lacpdu_header_t;
@@ -164,7 +158,7 @@ typedef struct bond_marker {
} bond_marker_t;
typedef struct bond_marker_header {
- struct ad_header ad_header;
+ struct ethhdr hdr;
struct bond_marker marker;
} bond_marker_header_t;
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -840,13 +840,11 @@ static int ad_lacpdu_send(struct port *p
lacpdu_header = (struct lacpdu_header *)skb_put(skb, length);
- memcpy(lacpdu_header->ad_header.destination_address.mac_addr_value,
- lacpdu_mcast_addr, ETH_ALEN);
+ memcpy(lacpdu_header->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
/* Note: source addres is set to be the member's PERMANENT address,
because we use it to identify loopback lacpdus in receive. */
- memcpy(lacpdu_header->ad_header.source_address.mac_addr_value,
- slave->perm_hwaddr, ETH_ALEN);
- lacpdu_header->ad_header.length_type = PKT_TYPE_LACPDU;
+ memcpy(lacpdu_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
+ lacpdu_header->hdr.h_proto = PKT_TYPE_LACPDU;
lacpdu_header->lacpdu = port->lacpdu; // struct copy
@@ -884,13 +882,11 @@ static int ad_marker_send(struct port *p
marker_header = (struct bond_marker_header *)skb_put(skb, length);
- memcpy(marker_header->ad_header.destination_address.mac_addr_value,
- lacpdu_mcast_addr, ETH_ALEN);
+ memcpy(marker_header->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
/* Note: source addres is set to be the member's PERMANENT address,
because we use it to identify loopback MARKERs in receive. */
- memcpy(marker_header->ad_header.source_address.mac_addr_value,
- slave->perm_hwaddr, ETH_ALEN);
- marker_header->ad_header.length_type = PKT_TYPE_LACPDU;
+ memcpy(marker_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
+ marker_header->hdr.h_proto = PKT_TYPE_LACPDU;
marker_header->marker = *marker; // struct copy
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 08/10] 802.3ad: remove public lacpdu_header
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
` (6 preceding siblings ...)
2008-12-23 22:01 ` [patch 07/10] 802.3ad: use standard ethhdr instead of ad_header Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-26 21:43 ` David Miller
2008-12-23 22:01 ` [patch 09/10] 802.3ad: remove public bond_marker_header Holger Eitzenberger
2008-12-23 22:01 ` [patch 10/10] 802.3ad: cleanup around lacpdu and bond_marker Holger Eitzenberger
9 siblings, 1 reply; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-remove-lacpdu_header.diff --]
[-- Type: text/plain, Size: 3232 bytes --]
It's strictly not necessary to define lacpdu_header in an include
file because it's only used in ad_lacpdu_send(). Remove that definition
and use an unnamed structure instead just for our purpose. Which also
gives a chance to use shorter names which are still descriptive.
As the port is left unmodified it's now const.
Also cleanup that function a bit by removing useless braces around
one-liners and turn C++ style comments into C comments.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -98,7 +98,6 @@ static const int ad_delta_in_ticks = (AD
static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
// ================= main 802.3ad protocol functions ==================
-static int ad_lacpdu_send(struct port *port);
static int ad_marker_send(struct port *port, struct bond_marker *marker);
static void ad_mux_machine(struct port *port);
static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port);
@@ -820,17 +819,19 @@ static inline void __update_lacpdu_from_
* Returns: 0 on success
* < 0 on error
*/
-static int ad_lacpdu_send(struct port *port)
+static int ad_lacpdu_send(const struct port *port)
{
struct slave *slave = port->slave;
struct sk_buff *skb;
- struct lacpdu_header *lacpdu_header;
- int length = sizeof(struct lacpdu_header);
+ struct {
+ struct ethhdr hdr;
+ struct lacpdu lacpdu;
+ } __packed *pdu;
+ int len = sizeof(*pdu);
- skb = dev_alloc_skb(length);
- if (!skb) {
+ skb = dev_alloc_skb(len);
+ if (skb == NULL)
return -ENOMEM;
- }
skb->dev = slave->dev;
skb_reset_mac_header(skb);
@@ -838,15 +839,15 @@ static int ad_lacpdu_send(struct port *p
skb->protocol = PKT_TYPE_LACPDU;
skb->priority = TC_PRIO_CONTROL;
- lacpdu_header = (struct lacpdu_header *)skb_put(skb, length);
+ pdu = (void *)skb_put(skb, len);
- memcpy(lacpdu_header->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
+ memcpy(pdu->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
/* Note: source addres is set to be the member's PERMANENT address,
because we use it to identify loopback lacpdus in receive. */
- memcpy(lacpdu_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
- lacpdu_header->hdr.h_proto = PKT_TYPE_LACPDU;
+ memcpy(pdu->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
+ pdu->hdr.h_proto = PKT_TYPE_LACPDU;
- lacpdu_header->lacpdu = port->lacpdu; // struct copy
+ memcpy(&pdu->lacpdu, &port->lacpdu, sizeof(pdu->lacpdu));
dev_queue_xmit(skb);
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -136,11 +136,6 @@ typedef struct lacpdu {
u8 reserved_50[50]; // = 0
} lacpdu_t;
-typedef struct lacpdu_header {
- struct ethhdr hdr;
- struct lacpdu lacpdu;
-} lacpdu_header_t;
-
// Marker Protocol Data Unit(PDU) structure(43.5.3.2 in the 802.3ad standard)
typedef struct bond_marker {
u8 subtype; // = 0x02 (marker PDU)
--
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 08/10] 802.3ad: remove public lacpdu_header
2008-12-23 22:01 ` [patch 08/10] 802.3ad: remove public lacpdu_header Holger Eitzenberger
@ 2008-12-26 21:43 ` David Miller
2008-12-26 21:45 ` David Miller
0 siblings, 1 reply; 21+ messages in thread
From: David Miller @ 2008-12-26 21:43 UTC (permalink / raw)
To: holger; +Cc: fubar, netdev
From: Holger Eitzenberger <holger@eitzenberger.org>
Date: Tue, 23 Dec 2008 23:01:09 +0100
> - skb = dev_alloc_skb(length);
> - if (!skb) {
> + skb = dev_alloc_skb(len);
> + if (skb == NULL)
"!skb" is preferred to the long-winded "skb == NULL", so
I'm tossing this patch and the rest which touch similar
areas.
Please don't mix cleanups and other changes unless you are %100
confident they will pass the most stringent review, otherwise your
patches will be rejected unnecessarily.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 08/10] 802.3ad: remove public lacpdu_header
2008-12-26 21:43 ` David Miller
@ 2008-12-26 21:45 ` David Miller
2008-12-29 16:15 ` Holger Eitzenberger
0 siblings, 1 reply; 21+ messages in thread
From: David Miller @ 2008-12-26 21:45 UTC (permalink / raw)
To: holger; +Cc: fubar, netdev
From: David Miller <davem@davemloft.net>
Date: Fri, 26 Dec 2008 13:43:49 -0800 (PST)
> From: Holger Eitzenberger <holger@eitzenberger.org>
> Date: Tue, 23 Dec 2008 23:01:09 +0100
>
> > - skb = dev_alloc_skb(length);
> > - if (!skb) {
> > + skb = dev_alloc_skb(len);
> > + if (skb == NULL)
>
> "!skb" is preferred to the long-winded "skb == NULL", so
> I'm tossing this patch and the rest which touch similar
> areas.
Also, I want to mention that your C++ comment conversions are not in
the proper coding style as well. You are doing:
/* something written
here */
instead of the proper:
/* something written
* here
*/
Just FYI...
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 08/10] 802.3ad: remove public lacpdu_header
2008-12-26 21:45 ` David Miller
@ 2008-12-29 16:15 ` Holger Eitzenberger
0 siblings, 0 replies; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-29 16:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fubar
On Fri, Dec 26, 2008 at 01:45:37PM -0800, David Miller wrote:
> > > - skb = dev_alloc_skb(length);
> > > - if (!skb) {
> > > + skb = dev_alloc_skb(len);
> > > + if (skb == NULL)
> >
> > "!skb" is preferred to the long-winded "skb == NULL", so
> > I'm tossing this patch and the rest which touch similar
> > areas.
>
> Also, I want to mention that your C++ comment conversions are not in
> the proper coding style as well. You are doing:
As I'm currently offline most of the time I'll resend once I'm back
home.
Thanks for your feedback!
/holger
^ permalink raw reply [flat|nested] 21+ messages in thread
* [patch 09/10] 802.3ad: remove public bond_marker_header
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
` (7 preceding siblings ...)
2008-12-23 22:01 ` [patch 08/10] 802.3ad: remove public lacpdu_header Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
2008-12-23 22:01 ` [patch 10/10] 802.3ad: cleanup around lacpdu and bond_marker Holger Eitzenberger
9 siblings, 0 replies; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-remove-bond_marker_header.diff --]
[-- Type: text/plain, Size: 3319 bytes --]
It's strictly not necessary to define bond_marker_header in an include
file because it's only used in ad_lacpdu_send(). Remove that definition
and use an unnamed structure instead just for our purpose. Which also
gives a chance to use shorter names which are still descriptive.
As the port and marker are left unmodified I've made them const.
As that function is defined before used I've left out the forward
declaration.
Also cleanup that function a bit by removing useless braces around
one-liners and turn C++ style comments into C comments.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -152,11 +152,6 @@ typedef struct bond_marker {
u8 reserved_90[90]; // = 0
} bond_marker_t;
-typedef struct bond_marker_header {
- struct ethhdr hdr;
- struct bond_marker marker;
-} bond_marker_header_t;
-
#pragma pack()
struct slave;
Index: bonding-2.6/drivers/net/bonding/bond_3ad.c
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.c
+++ bonding-2.6/drivers/net/bonding/bond_3ad.c
@@ -98,7 +98,6 @@ static const int ad_delta_in_ticks = (AD
static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
// ================= main 802.3ad protocol functions ==================
-static int ad_marker_send(struct port *port, struct bond_marker *marker);
static void ad_mux_machine(struct port *port);
static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port);
static void ad_tx_machine(struct port *port);
@@ -862,17 +861,20 @@ static int ad_lacpdu_send(const struct p
* Returns: 0 on success
* < 0 on error
*/
-static int ad_marker_send(struct port *port, struct bond_marker *marker)
+static int ad_marker_send(const struct port *port,
+ const struct bond_marker *marker)
{
struct slave *slave = port->slave;
struct sk_buff *skb;
- struct bond_marker_header *marker_header;
- int length = sizeof(struct bond_marker_header);
+ struct {
+ struct ethhdr hdr;
+ struct bond_marker marker;
+ } __packed *pdu;
+ int len = sizeof(*pdu);
- skb = dev_alloc_skb(length + 16);
- if (!skb) {
+ skb = dev_alloc_skb(len + 16);
+ if (skb == NULL)
return -ENOMEM;
- }
skb_reserve(skb, 16);
@@ -881,15 +883,15 @@ static int ad_marker_send(struct port *p
skb->network_header = skb->mac_header + ETH_HLEN;
skb->protocol = PKT_TYPE_LACPDU;
- marker_header = (struct bond_marker_header *)skb_put(skb, length);
+ pdu = (void *)skb_put(skb, len);
- memcpy(marker_header->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
+ memcpy(pdu->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
/* Note: source addres is set to be the member's PERMANENT address,
because we use it to identify loopback MARKERs in receive. */
- memcpy(marker_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
- marker_header->hdr.h_proto = PKT_TYPE_LACPDU;
+ memcpy(pdu->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
+ pdu->hdr.h_proto = PKT_TYPE_LACPDU;
- marker_header->marker = *marker; // struct copy
+ memcpy(&pdu->marker, marker, sizeof(pdu->marker));
dev_queue_xmit(skb);
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 10/10] 802.3ad: cleanup around lacpdu and bond_marker
2008-12-23 22:01 [patch 00/10] bonding updates for net-next-2.6 Holger Eitzenberger
` (8 preceding siblings ...)
2008-12-23 22:01 ` [patch 09/10] 802.3ad: remove public bond_marker_header Holger Eitzenberger
@ 2008-12-23 22:01 ` Holger Eitzenberger
9 siblings, 0 replies; 21+ messages in thread
From: Holger Eitzenberger @ 2008-12-23 22:01 UTC (permalink / raw)
To: David Miller; +Cc: Jay Vosburgh, netdev
[-- Attachment #1: bonding-3ad-remove-typedef-around-bond_marker.diff --]
[-- Type: text/plain, Size: 3566 bytes --]
Remove the typedef around lacpdu and bond_marker. Also turn C++
comments into C comments and some more or less pretty printing.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: bonding-2.6/drivers/net/bonding/bond_3ad.h
===================================================================
--- bonding-2.6.orig/drivers/net/bonding/bond_3ad.h
+++ bonding-2.6/drivers/net/bonding/bond_3ad.h
@@ -105,52 +105,56 @@ typedef enum {
#pragma pack(1)
-// Link Aggregation Control Protocol(LACP) data unit structure(43.4.2.2 in the 802.3ad standard)
-typedef struct lacpdu {
- u8 subtype; // = LACP(= 0x01)
+/* Link Aggregation Control Protocol (LACP) data unit structure
+ (43.4.2.2 in the 802.3ad standard) */
+struct lacpdu {
+ u8 subtype; /* LACP(= 0x01) */
u8 version_number;
- u8 tlv_type_actor_info; // = actor information(type/length/value)
- u8 actor_information_length; // = 20
+ u8 tlv_type_actor_info; /* actor information TLV */
+ u8 actor_information_length; /* = 20 */
__be16 actor_system_priority;
struct mac_addr actor_system;
__be16 actor_key;
__be16 actor_port_priority;
__be16 actor_port;
u8 actor_state;
- u8 reserved_3_1[3]; // = 0
- u8 tlv_type_partner_info; // = partner information
- u8 partner_information_length; // = 20
+ u8 reserved_3_1[3]; /* = 0 */
+ u8 tlv_type_partner_info; /* partner information */
+ u8 partner_information_length; /* = 20 */
__be16 partner_system_priority;
struct mac_addr partner_system;
__be16 partner_key;
__be16 partner_port_priority;
__be16 partner_port;
u8 partner_state;
- u8 reserved_3_2[3]; // = 0
- u8 tlv_type_collector_info; // = collector information
- u8 collector_information_length; // = 16
+ u8 reserved_3_2[3]; /* = 0 */
+ u8 tlv_type_collector_info; /* collector information */
+ u8 collector_information_length; /* = 16 */
__be16 collector_max_delay;
u8 reserved_12[12];
- u8 tlv_type_terminator; // = terminator
- u8 terminator_length; // = 0
- u8 reserved_50[50]; // = 0
-} lacpdu_t;
-
-// Marker Protocol Data Unit(PDU) structure(43.5.3.2 in the 802.3ad standard)
-typedef struct bond_marker {
- u8 subtype; // = 0x02 (marker PDU)
- u8 version_number; // = 0x01
- u8 tlv_type; // = 0x01 (marker information)
- // = 0x02 (marker response information)
- u8 marker_length; // = 0x16
- u16 requester_port; // The number assigned to the port by the requester
- struct mac_addr requester_system; // The requester's system id
- u32 requester_transaction_id; // The transaction id allocated by the requester,
- u16 pad; // = 0
- u8 tlv_type_terminator; // = 0x00
- u8 terminator_length; // = 0x00
- u8 reserved_90[90]; // = 0
-} bond_marker_t;
+ u8 tlv_type_terminator; /* terminator */
+ u8 terminator_length; /* = 0 */
+ u8 reserved_50[50]; /* = 0 */
+};
+
+/* Marker Protocol Data Unit (PDU) structure (43.5.3.2 in the 802.3ad
+ standard) */
+struct bond_marker {
+ u8 subtype; /* = 0x02 (marker PDU) */
+ u8 version_number; /* = 0x01 */
+ u8 tlv_type; /* 0x01: marker info,
+ 0x02: marker response info */
+ u8 marker_length; /* = 0x16 */
+ u16 requester_port; /* number assigned to the port
+ requester */
+ struct mac_addr requester_system; /* he requester's system id */
+ u32 requester_transaction_id; /* transaction id allocated by
+ requester */
+ u16 pad; /* = 00 */
+ u8 tlv_type_terminator; /* = 0x00 */
+ u8 terminator_length; /* = 0x00 */
+ u8 reserved_90[90]; /* = 0 */
+};
#pragma pack()
--
^ permalink raw reply [flat|nested] 21+ messages in thread