diff mbox

[5/6] mka: move structs {transmit, receive}_{sa, sc} to a common header

Message ID e49e059712bbef81768911b0cd007d823bdc4f33.1470913867.git.sd@queasysnail.net
State Changes Requested
Headers show

Commit Message

Sabrina Dubroca Aug. 15, 2016, 9:43 a.m. UTC 
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 src/common/ieee802_1x_defs.h  | 69 +++++++++++++++++++++++++++++++++++++++++++
 src/drivers/driver.h          |  3 ++
 src/pae/ieee802_1x_kay.h      |  5 ----
 src/pae/ieee802_1x_kay_i.h    | 62 --------------------------------------
 src/pae/ieee802_1x_secy_ops.h |  4 ---
 wpa_supplicant/driver_i.h     |  1 +
 6 files changed, 73 insertions(+), 71 deletions(-)

Comments

Jouni Malinen Aug. 22, 2016, 6:12 p.m. UTC | #1 
On Mon, Aug 15, 2016 at 11:43:43AM +0200, Sabrina Dubroca wrote:
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>

Could you please provide a bit more detailed commit message describing
why this would be done?

>  src/common/ieee802_1x_defs.h  | 69 +++++++++++++++++++++++++++++++++++++++++++

I'm not sure src/common/*.h is an appropriate location for this type of
definitions. I'd expect this file to include frame definitions and that
type of common defines/structures while the structs added here look more
like implementation specific data. What would those be shared with
(outside src/pae) and what is the "common" part here?
Sabrina Dubroca Aug. 24, 2016, 10:47 a.m. UTC | #2 
2016-08-22, 21:12:42 +0300, Jouni Malinen wrote:
> On Mon, Aug 15, 2016 at 11:43:43AM +0200, Sabrina Dubroca wrote:
> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> 
> Could you please provide a bit more detailed commit message describing
> why this would be done?

I reuse these structures in the next patch, which makes the driver ops
for MACsec cleaner, IMHO.  I will add this information to the commit
message if I update and resend this patch.


> >  src/common/ieee802_1x_defs.h  | 69 +++++++++++++++++++++++++++++++++++++++++++
> 
> I'm not sure src/common/*.h is an appropriate location for this type of
> definitions. I'd expect this file to include frame definitions and that
> type of common defines/structures while the structs added here look more
> like implementation specific data.

I see.  Well, these structs are more or less defined by the standard,
in that the standard describes the settings relevant for these items,
but I guess it could go either way.  I don't mind leaving them in
src/pae, it's your call.


> What would those be shared with (outside src/pae) and what is the
> "common" part here?

With drivers that support MACsec.  From your reply to patch 6, I
suppose that they at least need to be moved out of ieee802_1x_kay_i.h
if we want to use them in drivers.


Thanks for the feedback,
diff mbox

Patch

diff --git a/src/common/ieee802_1x_defs.h b/src/common/ieee802_1x_defs.h
index a0c1d1bfafc4..b4364fd4a70f 100644
--- a/src/common/ieee802_1x_defs.h
+++ b/src/common/ieee802_1x_defs.h
@@ -9,6 +9,8 @@ 
 #ifndef IEEE802_1X_DEFS_H
 #define IEEE802_1X_DEFS_H
 
+#include "utils/list.h"
+
 #define CS_ID_LEN		8
 #define CS_ID_GCM_AES_128	0x0080020001000001ULL
 #define CS_NAME_GCM_AES_128	"GCM-AES-128"
@@ -75,4 +77,71 @@  enum confidentiality_offset {
 #define DEFAULT_PRIO_GROUP_CA_MEMBER   0x70
 #define DEFAULT_PRIO_NOT_KEY_SERVER    0xFF
 
+struct ieee802_1x_mka_sci {
+	u8 addr[ETH_ALEN];
+	be16 port;
+};
+
+/* TransmitSC in IEEE Std 802.1AE-2006, Figure 10-6 */
+struct transmit_sc {
+	struct ieee802_1x_mka_sci sci; /* const SCI sci */
+	Boolean transmitting; /* bool transmitting (read only) */
+
+	struct os_time created_time; /* Time createdTime */
+
+	u8 encoding_sa; /* AN encodingSA (read only) */
+	u8 enciphering_sa; /* AN encipheringSA (read only) */
+
+	/* not defined data */
+	unsigned int channel;
+
+	struct dl_list list;
+	struct dl_list sa_list;
+};
+
+/* TransmitSA in IEEE Std 802.1AE-2006, Figure 10-6 */
+struct transmit_sa {
+	Boolean in_use; /* bool inUse (read only) */
+	u32 next_pn; /* PN nextPN (read only) */
+	struct os_time created_time; /* Time createdTime */
+
+	Boolean enable_transmit; /* bool EnableTransmit */
+
+	u8 an;
+	Boolean confidentiality;
+	struct data_key *pkey;
+
+	struct transmit_sc *sc;
+	struct dl_list list; /* list entry in struct transmit_sc::sa_list */
+};
+
+/* ReceiveSC in IEEE Std 802.1AE-2006, Figure 10-6 */
+struct receive_sc {
+	struct ieee802_1x_mka_sci sci; /* const SCI sci */
+	Boolean receiving; /* bool receiving (read only) */
+
+	struct os_time created_time; /* Time createdTime */
+
+	unsigned int channel;
+
+	struct dl_list list;
+	struct dl_list sa_list;
+};
+
+/* ReceiveSA in IEEE Std 802.1AE-2006, Figure 10-6 */
+struct receive_sa {
+	Boolean enable_receive; /* bool enableReceive */
+	Boolean in_use; /* bool inUse (read only) */
+
+	u32 next_pn; /* PN nextPN (read only) */
+	u32 lowest_pn; /* PN lowestPN (read only) */
+	u8 an;
+	struct os_time created_time;
+
+	struct data_key *pkey;
+	struct receive_sc *sc; /* list entry in struct receive_sc::sa_list */
+
+	struct dl_list list;
+};
+
 #endif /* IEEE802_1X_DEFS_H */
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index 8edbf5b24043..edd129ffdd62 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -21,6 +21,9 @@ 
 
 #include "common/defs.h"
 #include "common/ieee802_11_defs.h"
+#ifdef CONFIG_MACSEC
+#include "common/ieee802_1x_defs.h"
+#endif
 #include "utils/list.h"
 
 #define HOSTAPD_CHAN_DISABLED 0x00000001
diff --git a/src/pae/ieee802_1x_kay.h b/src/pae/ieee802_1x_kay.h
index c339d6d66334..08306b2987a2 100644
--- a/src/pae/ieee802_1x_kay.h
+++ b/src/pae/ieee802_1x_kay.h
@@ -29,11 +29,6 @@  struct ieee802_1x_mka_ki {
 	u32 kn;
 };
 
-struct ieee802_1x_mka_sci {
-	u8 addr[ETH_ALEN];
-	be16 port;
-};
-
 struct mka_key {
 	u8 key[MAX_KEY_LEN];
 	size_t len;
diff --git a/src/pae/ieee802_1x_kay_i.h b/src/pae/ieee802_1x_kay_i.h
index 622282e97c51..97eccf717993 100644
--- a/src/pae/ieee802_1x_kay_i.h
+++ b/src/pae/ieee802_1x_kay_i.h
@@ -74,68 +74,6 @@  struct data_key {
 	struct dl_list list;
 };
 
-/* TransmitSC in IEEE Std 802.1AE-2006, Figure 10-6 */
-struct transmit_sc {
-	struct ieee802_1x_mka_sci sci; /* const SCI sci */
-	Boolean transmitting; /* bool transmitting (read only) */
-
-	struct os_time created_time; /* Time createdTime */
-
-	u8 encoding_sa; /* AN encodingSA (read only) */
-	u8 enciphering_sa; /* AN encipheringSA (read only) */
-
-	/* not defined data */
-	unsigned int channel;
-
-	struct dl_list list;
-	struct dl_list sa_list;
-};
-
-/* TransmitSA in IEEE Std 802.1AE-2006, Figure 10-6 */
-struct transmit_sa {
-	Boolean in_use; /* bool inUse (read only) */
-	u32 next_pn; /* PN nextPN (read only) */
-	struct os_time created_time; /* Time createdTime */
-
-	Boolean enable_transmit; /* bool EnableTransmit */
-
-	u8 an;
-	Boolean confidentiality;
-	struct data_key *pkey;
-
-	struct transmit_sc *sc;
-	struct dl_list list; /* list entry in struct transmit_sc::sa_list */
-};
-
-/* ReceiveSC in IEEE Std 802.1AE-2006, Figure 10-6 */
-struct receive_sc {
-	struct ieee802_1x_mka_sci sci; /* const SCI sci */
-	Boolean receiving; /* bool receiving (read only) */
-
-	struct os_time created_time; /* Time createdTime */
-
-	unsigned int channel;
-
-	struct dl_list list;
-	struct dl_list sa_list;
-};
-
-/* ReceiveSA in IEEE Std 802.1AE-2006, Figure 10-6 */
-struct receive_sa {
-	Boolean enable_receive; /* bool enableReceive */
-	Boolean in_use; /* bool inUse (read only) */
-
-	u32 next_pn; /* PN nextPN (read only) */
-	u32 lowest_pn; /* PN lowestPN (read only) */
-	u8 an;
-	struct os_time created_time;
-
-	struct data_key *pkey;
-	struct receive_sc *sc; /* list entry in struct receive_sc::sa_list */
-
-	struct dl_list list;
-};
-
 struct macsec_ciphersuite {
 	u64 id;
 	char name[32];
diff --git a/src/pae/ieee802_1x_secy_ops.h b/src/pae/ieee802_1x_secy_ops.h
index f5057ee11958..120ca3c7e883 100644
--- a/src/pae/ieee802_1x_secy_ops.h
+++ b/src/pae/ieee802_1x_secy_ops.h
@@ -13,10 +13,6 @@ 
 #include "common/ieee802_1x_defs.h"
 
 struct ieee802_1x_kay_conf;
-struct receive_sa;
-struct transmit_sa;
-struct receive_sc;
-struct transmit_sc;
 
 int secy_init_macsec(struct ieee802_1x_kay *kay);
 int secy_deinit_macsec(struct ieee802_1x_kay *kay);
diff --git a/wpa_supplicant/driver_i.h b/wpa_supplicant/driver_i.h
index 220b7ba3ddca..676a9e77ae8e 100644
--- a/wpa_supplicant/driver_i.h
+++ b/wpa_supplicant/driver_i.h
@@ -10,6 +10,7 @@ 
 #define DRIVER_I_H
 
 #include "drivers/driver.h"
+#include "common/ieee802_1x_defs.h"
 
 /* driver_ops */
 static inline void * wpa_drv_init(struct wpa_supplicant *wpa_s,