MACsec IEEE 802.1AE implementation
Subsystems (3)
Sabrina Dubroca
maintainercover MACsec IEEE 802.1AE implementation
MACsec (IEEE 802.1AE [0]) is a protocol that provides security for wired ethernet LANs. MACsec offers two protection modes:
authentication only, or authenticated encryption.
MACsec defines "secure channels" that allow transmission from one node to one or more others. Communication on a channel is done over a succession of "secure associations", that each use a specific key. Secure associations are identified by their "association number" in the range 0..3. A secure association is retired when its 32-bit packet number would wrap, and the same association number can later be reused with a new key and packet number.
The standard mode of encryption is GCM AES with 128 bits keys, although an extension allows 256 bits keys [1] (not implemented in this submission).
When using MACsec, an extra header, called "SecTAG", is added between the ethernet header and the original payload:
+---------------------------------+----------------+----------------+ | (MACsec ethertype) | TCI_AN | SL | +---------------------------------+----------------+----------------+ | Packet Number | +-------------------------------------------------------------------+ | Secure Channel Identifier | | (optional) | +-------------------------------------------------------------------+
TCI_AN:
version
end_station
sci_present
scb
encrypted
changed_text
association_number (2 bits)
SL:
short_length (6 bits)
unused (2 bits)
The ethertype for the packet is set to 0x88E5, and the original ethertype becomes part of the secure payload, which may be encrypted. The ethernet header and the SecTAG are always transmitted in the clear, but are integrity-protected.
MACsec supports optional replay protection with a configurable replay window.
MACsec is designed to be used with the MKA extension to 802.1X (MACsec Key Agreement protocol) [2], which provides channel attribution and key distribution to the nodes, but can also be used with static keys getting fed manually by an administrator.
Optional (not supported yet) features:
- confidentiality offset: in encryption mode, part of the payload may be left unencrypted.
- choice of cipher suite: GCM AES with 256 bits has been standardised [1].
Implementation
A netdevice is created on top of a real device for each TX secure channel, like we do for VLANs. Multiple TX channels can be created on top of the same underlying device.
Several other approaches were considered for the RX path:
- dev_add_pack: doesn't work, because we want to filter out unprotected packets
- transparent mode: MACsec would be enabled directly on the real netdevice. For this, we cannot use a rx_handler directly because MACsec must be available for underlying devices enslaved in a bridge or in a bond, so we need a hook directly in __netif_receive_skb_core. This approach makes it harder to filter non-encrypted packets on RX without forcing the user to setup some rules, so the "transparent" mode is not so transparent after all. It also makes TX more complex than with a dedicated netdevice.
One issue with the proposed implementation is that the qdisc layer for the real device operates on already encrypted packets.
Netlink API
This is currently a mix of rtnetlink (to create the device and set up the TX channel) and genl (for RX channels, secure associations and their keys). genl provides clean demultiplexing of the {TX,RX}{SC,SA} commands.
Use cases
The normal use case is wired LANs, including veth and slave devices for bonding/teaming or bridges.
MACsec can also be used on any device that makes a full ethernet header visible, for example VXLAN.
The VXLAN+MACsec setup would be:
hypervisor | virtual machine
<real_dev>---<VXLAN>---|---<dev>---<macsec_dev>
And the packets would look like this:
| eth | IP | UDP | VXLAN | eth | MACsec | IP | ... | MACsec ICV |
One benefit on this approach to encryption in the cloud is that the payload is encrypted by the tenant, not by the tunnel provider, thus the tenant has full control over the keys.
Changes from RFCv2:
- fix ENCODING_SA param validation
- add parent link to netlink ifdumps
Changes from RFCv1 (details in individual patches):
- addressed comments from Florian and Paolo + kbuild robot
- also perform post-decrypt handling after crypto callback
- fixed ->dellink behavior
Future plans:
- offload to hardware, on nics that support it
- implement optional features
[0] http://standards.ieee.org/getieee802/download/802.1AE-2006.pdf
[1] http://standards.ieee.org/getieee802/download/802.1AEbn-2011.pdf
[2] http://standards.ieee.org/getieee802/download/802.1X-2010.pdf
[3] link to RFCv1: http://www.spinics.net/lists/netdev/msg358151.html
[4] link to RFCv2: http://www.spinics.net/lists/netdev/msg362389.html
Sabrina Dubroca (3):
uapi: add MACsec bits
net: add MACsec netdevice priv_flags and helper
macsec: introduce IEEE 802.1AE driver
drivers/net/Kconfig | 7 + drivers/net/Makefile | 1 + drivers/net/macsec.c | 3037 ++++++++++++++++++++++++++++++++++++++++ include/linux/netdevice.h | 8 + include/uapi/linux/Kbuild | 1 + include/uapi/linux/if_ether.h | 1 + include/uapi/linux/if_link.h | 21 + include/uapi/linux/if_macsec.h | 165 +++ 8 files changed, 3241 insertions(+) create mode 100644 drivers/net/macsec.c create mode 100644 include/uapi/linux/if_macsec.h
Signature · 1 line
2.7.2
Sabrina Dubroca
maintainerPATCH 1/3 uapi: add MACsec bits
---
v1:
- rebase
RFCv2:
- add include, detected by the kbuild robot
- split off the attributes for RXSC and SA from enum macsec_attrs
--- include/uapi/linux/Kbuild | 1 + include/uapi/linux/if_ether.h | 1 + include/uapi/linux/if_link.h | 21 ++++++ include/uapi/linux/if_macsec.h | 165 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 188 insertions(+) create mode 100644 include/uapi/linux/if_macsec.h
include/uapi/linux/Kbuild +1 -0
@@ -173,6 +173,7 @@ header-y += if_hippi.h header-y += if_infiniband.h header-y += if_link.h header-y += if_ltalk.h +header-y += if_macsec.h header-y += if_packet.h header-y += if_phonet.h header-y += if_plip.h
include/uapi/linux/if_ether.h +1 -0
@@ -83,6 +83,7 @@#define ETH_P_8021AD 0x88A8/* 802.1ad Service VLAN */ #define ETH_P_802_EX1 0x88B5/* 802.1 Local Experimental 1. */ #define ETH_P_TIPC 0x88CA/* TIPC */ +#define ETH_P_MACSEC 0x88E5/* 802.1ae MACsec */ #define ETH_P_8021AH 0x88E7/* 802.1ah Backbone Service Tag */ #define ETH_P_MVRP 0x88F5/* 802.1Q MVRP */ #define ETH_P_1588 0x88F7/* IEEE 1588 Timesync */
include/uapi/linux/if_link.h +21 -0
@@ -413,6 +413,27 @@ enum {#define IFLA_VRF_PORT_MAX (__IFLA_VRF_PORT_MAX - 1) +/* MACSEC section */ +enum { +IFLA_MACSEC_UNSPEC , +IFLA_MACSEC_SCI , +IFLA_MACSEC_PORT , +IFLA_MACSEC_ICV_LEN , +IFLA_MACSEC_CIPHER_SUITE , +IFLA_MACSEC_WINDOW , +IFLA_MACSEC_ENCODING_SA , +IFLA_MACSEC_ENCRYPT , +IFLA_MACSEC_PROTECT , +IFLA_MACSEC_INC_SCI , +IFLA_MACSEC_ES , +IFLA_MACSEC_SCB , +IFLA_MACSEC_REPLAY_PROTECT , +IFLA_MACSEC_VALIDATION , + __IFLA_MACSEC_MAX, +}; + +#define IFLA_MACSEC_MAX (__IFLA_MACSEC_MAX - 1) +/* IPVLAN section */ enum {IFLA_IPVLAN_UNSPEC ,
include/uapi/linux/if_macsec.h new file +165 -0
165 lines not on this page · raw message
Signature · 1 line
2.7.2
Re: [PATCH net-next 1/3] uapi: add MACsec bits
Quoted 1 line · Sabrina Dubroca
+++
b/include/uapi/linux/if_macsec.h
Some bits of documentation in this file, regarding all the various operations and attributes, might be nice. At least the netlink types?
E.g., given this:
Quoted 5 lines
include/uapi/linux/if_macsec.h new file +4
-0+#define DEFAULT_CIPHER_NAME "GCM-AES-128" +#define DEFAULT_CIPHER_ID 0x0080020001000001ULL +#define DEFAULT_CIPHER_ALT 0x0080C20001000001ULL +enum macsec_attrs {
[...]
include/uapi/linux/if_macsec.h line 36
Quoted 1 line
include/uapi/linux/if_macsec.h new file +1
-0+ MACSEC_ATTR_CIPHER_SUITE,
should that be the ID, the alternate ID, or the string?
include/uapi/linux/if_macsec.h line 42
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+MACSEC_ATTR_ICV_LEN , +MACSEC_TXSA_LIST , +MACSEC_RXSC_LIST , +MACSEC_TXSC_STATS , +MACSEC_SECY_STATS , + MACSEC_ATTR_PROTECT,
This seems a bit inconsistent, MACSEC_ATTR_* vs. MACSEC_*?
include/uapi/linux/if_macsec.h line 60
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+enum macsec_sa_list_attrs { +MACSEC_SA_LIST_UNSPEC , +MACSEC_SA , + __MACSEC_ATTR_SA_LIST_MAX, +MACSEC_ATTR_SA_LIST_MAX = __MACSEC_ATTR_SA_LIST_MAX -1 , +};
Again, without documentation, it seems odd to have an enum with just a single useful entry? If you just wanted an array you don't need this at all? The netlink nesting properties could be specified somewhere.
include/uapi/linux/if_macsec.h line 64
Quoted 3 lines
include/uapi/linux/if_macsec.h new file +3
-0+enum macsec_rxsc_list_attrs { +MACSEC_RXSC_LIST_UNSPEC , +MACSEC_RXSC ,
similarly here
include/uapi/linux/if_macsec.h line 74
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+enum macsec_rxsc_attrs { +MACSEC_ATTR_SC_UNSPEC , +/* use the same value to allow generic helper function, see +* get_*_from_nl in drivers/net/macsec.c */ +MACSEC_ATTR_SC_IFINDEX =MACSEC_ATTR_IFINDEX , +MACSEC_ATTR_SC_SCI =MACSEC_ATTR_SCI ,
This seems odd, this must be nested inside the top-level attributes since it's a single genl family, so why not use the top-level attributes to start with?
If you need multiple you can use dump with multiple messages.
include/uapi/linux/if_macsec.h line 88
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+enum macsec_sa_attrs { +MACSEC_ATTR_SA_UNSPEC , +/* use the same value to allow generic helper function, see +* get_*_from_nl in drivers/net/macsec.c */ +MACSEC_ATTR_SA_IFINDEX =MACSEC_ATTR_IFINDEX , +MACSEC_ATTR_SA_SCI =MACSEC_ATTR_SCI ,
likewise here
include/uapi/linux/if_macsec.h line 119
Quoted 7 lines
include/uapi/linux/if_macsec.h new file +7
-0+enum validation_type { +MACSEC_VALIDATE_DISABLED =0 , +MACSEC_VALIDATE_CHECK =1 , +MACSEC_VALIDATE_STRICT =2 , + __MACSEC_VALIDATE_MAX, +}; +#define MACSEC_VALIDATE_MAX (__MACSEC_VALIDATE_MAX - 1)
everywhere else you put that into the enum, why not here?
include/uapi/linux/if_macsec.h line 132
Quoted 12 lines
include/uapi/linux/if_macsec.h new file +12
-0+struct macsec_rx_sc_stats { +__u64 InOctetsValidated; +__u64 InOctetsDecrypted; +__u64 InPktsUnchecked; +__u64 InPktsDelayed; +__u64 InPktsOK; +__u64 InPktsInvalid; +__u64 InPktsLate; +__u64 InPktsNotValid; +__u64 InPktsNotUsingSA; +__u64 InPktsUnusedSA; +};
It might be worth splitting those into attributes so that, if some hardware offload can't provide all of the counters in the future, they can just be left out of the netlink message?
johannes
Sabrina Dubroca
maintainerRe: [PATCH net-next 1/3] uapi: add MACsec bits
Quoted 6 lines · Johannes Berg
Quoted 1 line · Sabrina Dubroca
+++
b/include/uapi/linux/if_macsec.hSome bits of documentation in this file, regarding all the various operations and attributes, might be nice. At least the netlink types?
ok. Most of them are already indicated in the policies, but I can add some comments here.
Quoted 11 lines
E.g., given this:
Quoted 5 lines
include/uapi/linux/if_macsec.h new file +4
-0+#define DEFAULT_CIPHER_NAME "GCM-AES-128" +#define DEFAULT_CIPHER_ID 0x0080020001000001ULL +#define DEFAULT_CIPHER_ALT 0x0080C20001000001ULL +enum macsec_attrs {[...]
include/uapi/linux/if_macsec.h line 36
Quoted 1 line
include/uapi/linux/if_macsec.h new file +1
-0+ MACSEC_ATTR_CIPHER_SUITE,should that be the ID, the alternate ID, or the string?
uh, the string is never actually used, I could get rid of it.
Quoted 8 lines
include/uapi/linux/if_macsec.h line 42
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+MACSEC_ATTR_ICV_LEN , +MACSEC_TXSA_LIST , +MACSEC_RXSC_LIST , +MACSEC_TXSC_STATS , +MACSEC_SECY_STATS , + MACSEC_ATTR_PROTECT,This seems a bit inconsistent, MACSEC_ATTR_* vs. MACSEC_*?
Only the MACSEC_ATTR_* can be set, the others are just for dumping.
Quoted 10 lines
include/uapi/linux/if_macsec.h line 60
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+enum macsec_sa_list_attrs { +MACSEC_SA_LIST_UNSPEC , +MACSEC_SA , + __MACSEC_ATTR_SA_LIST_MAX, +MACSEC_ATTR_SA_LIST_MAX = __MACSEC_ATTR_SA_LIST_MAX -1 , +};Again, without documentation, it seems odd to have an enum with just a single useful entry? If you just wanted an array you don't need this at all? The netlink nesting properties could be specified somewhere.
Yes, in dump_secy(), I nest the TXSA_LIST, and then each SA underneath it. I'm not sure how that would work without the list. Can you have an array without the dummy level of nesting?
Quoted 28 lines
include/uapi/linux/if_macsec.h line 64
Quoted 3 lines
include/uapi/linux/if_macsec.h new file +3
-0+enum macsec_rxsc_list_attrs { +MACSEC_RXSC_LIST_UNSPEC , +MACSEC_RXSC ,similarly here
include/uapi/linux/if_macsec.h line 74
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+enum macsec_rxsc_attrs { +MACSEC_ATTR_SC_UNSPEC , +/* use the same value to allow generic helper function, see +* get_*_from_nl in drivers/net/macsec.c */ +MACSEC_ATTR_SC_IFINDEX =MACSEC_ATTR_IFINDEX , +MACSEC_ATTR_SC_SCI =MACSEC_ATTR_SCI ,This seems odd, this must be nested inside the top-level attributes since it's a single genl family, so why not use the top-level attributes to start with?
If you need multiple you can use dump with multiple messages.
include/uapi/linux/if_macsec.h line 88
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+enum macsec_sa_attrs { +MACSEC_ATTR_SA_UNSPEC , +/* use the same value to allow generic helper function, see +* get_*_from_nl in drivers/net/macsec.c */ +MACSEC_ATTR_SA_IFINDEX =MACSEC_ATTR_IFINDEX , +MACSEC_ATTR_SA_SCI =MACSEC_ATTR_SCI ,likewise here
Quoted 9 lines
include/uapi/linux/if_macsec.h line 119
Quoted 7 lines
include/uapi/linux/if_macsec.h new file +7
-0+enum validation_type { +MACSEC_VALIDATE_DISABLED =0 , +MACSEC_VALIDATE_CHECK =1 , +MACSEC_VALIDATE_STRICT =2 , + __MACSEC_VALIDATE_MAX, +}; +#define MACSEC_VALIDATE_MAX (__MACSEC_VALIDATE_MAX - 1)everywhere else you put that into the enum, why not here?
Will fix.
Quoted 8 lines
Quoted 8 lines
include/uapi/linux/if_macsec.h new file +8
-0+struct macsec_rx_sc_stats { +__u64 InOctetsValidated; +__u64 InOctetsDecrypted; +__u64 InPktsUnchecked; +__u64 InPktsDelayed; +__u64 InPktsOK; +__u64 InPktsInvalid; +__u64 InPktsLate;
Quoted 8 lines
include/uapi/linux/if_macsec.h line 132
Quoted 4 lines
include/uapi/linux/if_macsec.h new file +4
-0+__u64 InPktsNotValid; +__u64 InPktsNotUsingSA; +__u64 InPktsUnusedSA; +};It might be worth splitting those into attributes so that, if some hardware offload can't provide all of the counters in the future, they can just be left out of the netlink message?
These stats are defined by the standard, but marked optional. A hardware device that doesn't implement some stat could just ignore it and export 0.
I don't have a strong opinion about this.
Thanks,
Signature · 1 line
Sabrina
Re: [PATCH net-next 1/3] uapi: add MACsec bits
Quoted 11 lines · Sabrina Dubroca
Quoted 9 lines
Quoted 5 lines
include/uapi/linux/if_macsec.h new file +4
-0+#define DEFAULT_CIPHER_NAME "GCM-AES-128" +#define DEFAULT_CIPHER_ID 0x0080020001000001ULL +#define DEFAULT_CIPHER_ALT 0x0080C20001000001ULL +enum macsec_attrs {[...]
include/uapi/linux/if_macsec.h line 36
Quoted 1 line
include/uapi/linux/if_macsec.h new file +1
-0+ MACSEC_ATTR_CIPHER_SUITE,should that be the ID, the alternate ID, or the string?
uh, the string is never actually used, I could get rid of it.
Heh, ok. I was actually wondering about the string but didn't look up where/if it was used :)
Quoted 10 lines
Quoted 8 lines
include/uapi/linux/if_macsec.h line 42
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+MACSEC_ATTR_ICV_LEN , +MACSEC_TXSA_LIST , +MACSEC_RXSC_LIST , +MACSEC_TXSC_STATS , +MACSEC_SECY_STATS , + MACSEC_ATTR_PROTECT,This seems a bit inconsistent, MACSEC_ATTR_* vs. MACSEC_*?
Only the MACSEC_ATTR_* can be set, the others are just for dumping.
Makes sense too.
I tend to prefer the names having a consistent prefix to indicate the enum they're used in, which indicates the nesting level in nl80211 etc. and makes it easier to figure out in the code that they're used correctly (since accidentally mixing enums will give no warnings), but that's just personal preference I guess.
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
include/uapi/linux/if_macsec.h new file +4
-0+enum macsec_sa_list_attrs { +MACSEC_SA_LIST_UNSPEC , +MACSEC_SA , + __MACSEC_ATTR_SA_LIST_MAX,Quoted 8 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ MACSEC_ATTR_SA_LIST_MAX = __MACSEC_ATTR_SA_LIST_MAX - 1, +};Again, without documentation, it seems odd to have an enum with just a
single useful entry? If you just wanted an array you don't need this at
all? The netlink nesting properties could be specified somewhere.
Quoted 4 lines
Yes, in
dump_secy(), I nest theTXSA_LIST, and then each SA underneath
it. I'm not sure how that would work without the list. Can you have an array without the dummy level of nesting?
So, if I understand correctly, your message would be [
..., /* e.g. IFINDEX, perhaps */ TXSA_LIST -> [
MACSEC_SA -> [
MACSEC_ATTR_SA_AN -> ..., MACSEC_ATTR_SA_PN -> ...
],
MACSEC_SA -> [...],
MACSEC_SA -> [...],
...
],
]
right? That seems pretty odd to me, usually the same nesting level in netlink shouldn't contain the same attribute multiple times, as I understand it.
I *think* the way we do this in nl80211 is more customary, it would be like this in your case (without defining the sa_list_attrs enum):
[
..., /* e.g. IFINDEX, perhaps */
TXSA_LIST -> [
1 -> [
MACSEC_ATTR_SA_AN -> ...,
MACSEC_ATTR_SA_PN -> ...
],
2 -> [...],
3 -> [...],
...
],
]
See, for example, nl80211_send_wowlan_patterns() which nests like this:
[
NL80211_WOWLAN_TRIG_PKT_PATTERN -> [
1 -> [
NL80211_PKTPAT_MASK -> ...,
NL80211_PKTPAT_PATTERN -> ...,
NL80211_PKTPAT_OFFSET -> ...,
],
2 -> [...],
...
]
]
Quoted 3 lines
These stats are defined by the standard, but marked optional. A hardware device that doesn't implement some stat could just ignore it and export 0.
Fair enough. I tend to think there could be a difference between knowing the value was 0 and knowing it wasn't provided, particularly for the "exceptions" that you'd hope are mostly 0 under good operating conditions, but I don't have a strong opinion about these or, obviously, any idea about whether hardware might not be able to provide them.
johannes
Sabrina Dubroca
maintainerRe: [PATCH net-next 1/3] uapi: add MACsec bits
Hi Johannes,
Quoted 12 lines · Johannes Berg
Quoted 10 lines · Sabrina Dubroca
Quoted 8 lines
include/uapi/linux/if_macsec.h line 42
Quoted 6 lines
include/uapi/linux/if_macsec.h new file +6
-0+MACSEC_ATTR_ICV_LEN , +MACSEC_TXSA_LIST , +MACSEC_RXSC_LIST , +MACSEC_TXSC_STATS , +MACSEC_SECY_STATS , + MACSEC_ATTR_PROTECT,This seems a bit inconsistent, MACSEC_ATTR_* vs. MACSEC_*?
Only the MACSEC_ATTR_* can be set, the others are just for dumping.
Quoted 7 lines
Makes sense too.
I tend to prefer the names having a consistent prefix to indicate the enum they're used in, which indicates the nesting level in nl80211 etc. and makes it easier to figure out in the code that they're used correctly (since accidentally mixing enums will give no warnings), but that's just personal preference I guess.
I see. I like the verification aspect, I'm adapting the enums now.
Quoted 33 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
include/uapi/linux/if_macsec.h new file +4
-0+enum macsec_sa_list_attrs { +MACSEC_SA_LIST_UNSPEC , +MACSEC_SA , + __MACSEC_ATTR_SA_LIST_MAX,Quoted 8 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ MACSEC_ATTR_SA_LIST_MAX = __MACSEC_ATTR_SA_LIST_MAX - 1, +};Again, without documentation, it seems odd to have an enum with just a
single useful entry? If you just wanted an array you don't need this at
all? The netlink nesting properties could be specified somewhere.Quoted 4 lines
Yes, in
dump_secy(), I nest theTXSA_LIST, and then each SA underneath
it. I'm not sure how that would work without the list. Can you have an array without the dummy level of nesting?So, if I understand correctly, your message would be [
..., /* e.g. IFINDEX, perhaps */TXSA_LIST-> [
MACSEC_SA -> [
MACSEC_ATTR_SA_AN -> ...,MACSEC_ATTR_SA_PN-> ...
],
MACSEC_SA -> [...],
MACSEC_SA -> [...],
...
],
]
Quoted 3 lines
right? That seems pretty odd to me, usually the same nesting level in netlink shouldn't contain the same attribute multiple times, as I understand it.
Well, it worked ;)
Quoted 18 lines
I *think* the way we do this in nl80211 is more customary, it would be like this in your case (without defining the
sa_list_attrsenum):[
..., /* e.g. IFINDEX, perhaps */
TXSA_LIST -> [
1 -> [
MACSEC_ATTR_SA_AN -> ...,
MACSEC_ATTR_SA_PN -> ...
],
2 -> [...],
3 -> [...],
...
],
]See, for example, nl80211_send_wowlan_patterns() which nests like this:
Quoted 11 lines
[
NL80211_WOWLAN_TRIG_PKT_PATTERN -> [
1 -> [
NL80211_PKTPAT_MASK -> ...,
NL80211_PKTPAT_PATTERN -> ...,
NL80211_PKTPAT_OFFSET -> ...,
],
2 -> [...],
...
]
]
Ah, ok. I'm using this now, no more dummy enum. And thanks for the pointer!
Quoted 10 lines
Quoted 3 lines
These stats are defined by the standard, but marked optional. A hardware device that doesn't implement some stat could just ignore it and export 0.
Fair enough. I tend to think there could be a difference between knowing the value was 0 and knowing it wasn't provided, particularly for the "exceptions" that you'd hope are mostly 0 under good operating conditions, but I don't have a strong opinion about these or, obviously, any idea about whether hardware might not be able to provide them.
Hmm, yeah, that makes sense. I'll think about it a bit more, maybe I will change that before I resubmit. The separate attributes would also help a bit in case we need to add more stats.
Thanks,
Signature · 1 line
Sabrina
Sabrina Dubroca
maintainerPATCH 2/3 net: add MACsec netdevice priv_flags and helper
In reply to Sabrina Dubroca---
v1: rebase
RFCv2: add kerneldoc comment, detected by kbuild robot
--- include/linux/netdevice.h | 8 ++++++++ 1 file changed, 8 insertions(+)
include/linux/netdevice.h +8 -0
@@ -1325,6 +1325,7 @@ struct net_device_ops {* @IFF_RXFH_CONFIGURED: device has had Rx Flow indirection table configured * @IFF_PHONY_HEADROOM: the headroom value is controlled by an external * entity (i.e. the master device for bridged veth) +* @IFF_MACSEC: device is a MACsec device */ enum netdev_priv_flags {IFF_802_1Q_VLAN =1 <<0 ,
@@ -1354,6 +1355,7 @@ enum netdev_priv_flags {IFF_TEAM =1 <<24 ,IFF_RXFH_CONFIGURED =1 <<25 ,IFF_PHONY_HEADROOM =1 <<26 , + IFF_MACSEC = 1<<27, };#define IFF_802_1Q_VLAN IFF_802_1Q_VLAN
@@ -1382,6 +1384,7 @@ enum netdev_priv_flags {#define IFF_L3MDEV_SLAVE IFF_L3MDEV_SLAVE#define IFF_TEAM IFF_TEAM#define IFF_RXFH_CONFIGURED IFF_RXFH_CONFIGURED +#define IFF_MACSEC IFF_MACSEC/** * struct net_device - The DEVICE structure.
@@ -4025,6 +4028,11 @@ static inline void skb_gso_error_unwind(struct sk_buff *skb, __be16 protocol, skb->mac_len = mac_len; } +static inline bool netif_is_macsec (const struct net_device *dev) +{ +return dev->priv_flags &IFF_MACSEC ; +} +static inline bool netif_is_macvlan (const struct net_device *dev) {return dev->priv_flags &IFF_MACVLAN ;
Signature · 1 line
2.7.2
Sabrina Dubroca
maintainerPATCH 3/3 macsec: introduce IEEE 802.1AE driver
In reply to Sabrina DubrocaThis is an implementation of MACsec/IEEE 802.1AE. This driver provides authentication and encryption of traffic in a LAN, typically with GCM-AES-128, and optional replay protection.
http://standards.ieee.org/getieee802/download/802.1AE-2006.pdf
---
v1:
- fix ENCODING_SA param validation
- add parent link to netlink ifdumps
RFCv2:
- rcu fixes pointed out by Florian
- get rid of magic numbers in shortlength field validation, use a nice helper function instead
- extract post-decrypt handling from macsec_handle_frame to reuse it in the decrypt callback
- needed headroom shouldn't include hard_header len, pointed out by Paolo
- added refcounting on macsec_rx_sc
- fixed ->dellink behavior
--- drivers/net/Kconfig | 7 + drivers/net/Makefile | 1 + drivers/net/macsec.c | 3037 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 3045 insertions(+) create mode 100644 drivers/net/macsec.c
drivers/net/Kconfig +7 -0
@@ -193,6 +193,13 @@ config GENEVE To compile this driver as a module, choose M here: the module will be called geneve. +config MACSEC + tristate "IEEE 802.1AE MAC-level encryption (MACsec)" + select CRYPTO_AES + select CRYPTO_GCM + ---help--- + MACsec is an encryption standard for Ethernet. + config NETCONSOLE tristate "Network console logging support" ---help---
drivers/net/Makefile +1 -0
@@ -10,6 +10,7 @@ obj-$(CONFIG_IPVLAN) += ipvlan/ obj-$(CONFIG_DUMMY) += dummy.o obj-$(CONFIG_EQUALIZER) += eql.o obj-$(CONFIG_IFB) += ifb.o +obj-$(CONFIG_MACSEC) += macsec.o obj-$(CONFIG_MACVLAN) += macvlan.o obj-$(CONFIG_MACVTAP) += macvtap.o obj-$(CONFIG_MII) += mii.o
drivers/net/macsec.c new file +3037 -0
2,980 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.7.2
Part of the diff is not shown here. View the raw message.
David Miller
maintainerRe: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver
From: Sabrina Dubroca <[email protected]>
Date: Mon, 7 Mar 2016 18:12:40 +0100
Quoted 862 lines
drivers/net/Kconfig +7
-0@@ -193,6 +193,13 @@ config GENEVE To compile this driver as a module, choose M here: the module will be called geneve. +config MACSEC + tristate "IEEE 802.1AE MAC-level encryption (MACsec)" + select CRYPTO_AES + select CRYPTO_GCM + ---help--- + MACsec is an encryption standard for Ethernet. + config NETCONSOLE tristate "Network console logging support" ---help---drivers/net/Makefile +1
-0@@ -10,6 +10,7 @@ obj-$(CONFIG_IPVLAN) += ipvlan/ obj-$(CONFIG_DUMMY) += dummy.o obj-$(CONFIG_EQUALIZER) += eql.o obj-$(CONFIG_IFB) += ifb.o +obj-$(CONFIG_MACSEC) += macsec.o obj-$(CONFIG_MACVLAN) += macvlan.o obj-$(CONFIG_MACVTAP) += macvtap.o obj-$(CONFIG_MII) += mii.odrivers/net/macsec.c new file +826
-0@@ -0,0 +1,3037 @@ +/* +* drivers/net/macsec.c - MACsec device +* +* Copyright (c) 2015 Sabrina Dubroca <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +*/ + +#include <linux/types.h> +#include <linux/skbuff.h> +#include <linux/socket.h> +#include <linux/module.h> +#include <crypto/aead.h> +#include <linux/etherdevice.h> +#include <linux/rtnetlink.h> +#include <net/genetlink.h> +#include <net/sock.h> + +#include <uapi/linux/if_macsec.h> + +typedef u64 __bitwise sci_t ; + +#define MACSEC_SCI_LEN 8 + +/* SecTAG length = macsec_eth_header without the optional SCI */ +#define MACSEC_TAG_LEN 6 + +struct macsec_eth_header { +struct ethhdr eth; +/* SecTAG */ +u8 tci_an; +#if defined(__LITTLE_ENDIAN_BITFIELD) +u8 short_length:6 , + unused:2 ; +#elif defined(__BIG_ENDIAN_BITFIELD) +u8 unused:2 , + short_length:6 ; +#else +#error "Please fix <asm/byteorder.h>" +#endif +__be32 packet_number; +u8 secure_channel_id[8 ];/* optional */ +} __packed; + +#define MACSEC_TCI_VERSION 0x80 +#define MACSEC_TCI_ES 0x40/* end station */ +#define MACSEC_TCI_SC 0x20/* SCI present */ +#define MACSEC_TCI_SCB 0x10/* epon */ +#define MACSEC_TCI_E 0x08/* encryption */ +#define MACSEC_TCI_C 0x04/* changed text */ +#define MACSEC_AN_MASK 0x03/* association number */ +#define MACSEC_TCI_CONFID (MACSEC_TCI_E | MACSEC_TCI_C) + +#define MACSEC_SHORTLEN_THR 48 + +#define GCM_AES_IV_LEN 12 +#define DEFAULT_ICV_LEN 16 + +#define for_each_rxsc (secy, sc) \ + for (sc = rcu_dereference_bh(secy->rx_sc); \ + sc; \ + sc = rcu_dereference_bh(sc->next)) +#define for_each_rxsc_rtnl (secy, sc) \ + for (sc = rtnl_dereference(secy->rx_sc); \ + sc; \ + sc = rtnl_dereference(sc->next)) + +struct gcm_iv { +union { +u8 secure_channel_id[8 ]; +sci_t sci; + }; +__be32 pn; +}; + +/** +* struct macsec_key - SA key +* @id user-provided key identifier +* @tfm crypto struct, key storage +*/ +struct macsec_key { +u64 id; +struct crypto_aead *tfm; +}; + +/** +* struct macsec_rx_sa - receive secure association +* @active +* @next_pn packet number expected for the next packet +* @lock protects next_pn manipulations +* @key key structure +* @stats per-SA stats +*/ +struct macsec_rx_sa { +bool active; +u32 next_pn; +spinlock_t lock; +struct macsec_key key; +struct macsec_rx_sa_stats __percpu *stats; +struct macsec_rx_sc *sc; +atomic_t refcnt; +struct rcu_head rcu; +}; + +struct pcpu_rx_sc_stats { +struct macsec_rx_sc_stats stats; +struct u64_stats_sync syncp; +}; + +/** +* struct macsec_rx_sc - receive secure channel +* @sci secure channel identifier for this SC +* @active channel is active +* @sa array of secure associations +* @stats per-SC stats +*/ +struct macsec_rx_sc { +struct macsec_rx_sc __rcu *next; +sci_t sci; +bool active; +struct macsec_rx_sa __rcu *sa[4 ]; +struct pcpu_rx_sc_stats __percpu *stats; +atomic_t refcnt; +struct rcu_head rcu_head; +}; + +/** +* struct macsec_tx_sa - transmit secure association +* @active +* @next_pn packet number to use for the next packet +* @lock protects next_pn manipulations +* @key key structure +* @stats per-SA stats +*/ +struct macsec_tx_sa { +bool active; +u32 next_pn; +spinlock_t lock; +struct macsec_key key; +struct macsec_tx_sa_stats __percpu *stats; +atomic_t refcnt; +struct rcu_head rcu; +}; + +struct pcpu_tx_sc_stats { +struct macsec_tx_sc_stats stats; +struct u64_stats_sync syncp; +}; + +/** +* struct macsec_tx_sc - transmit secure channel +* @active +* @encoding_sa association number of the SA currently in use +* @encrypt encrypt packets on transmit, or authenticate only +* @send_sci always include the SCI in the SecTAG +* @end_station +* @scb single copy broadcast flag +* @sa array of secure associations +* @stats stats for this TXSC +*/ +struct macsec_tx_sc { +bool active; +u8 encoding_sa; +bool encrypt; +bool send_sci; +bool end_station; +bool scb; +struct macsec_tx_sa __rcu *sa[4 ]; +struct pcpu_tx_sc_stats __percpu *stats; +}; + +#define MACSEC_VALIDATE_DEFAULT MACSEC_VALIDATE_STRICT + +/** +* struct macsec_secy - MACsec Security Entity +* @netdev netdevice for this SecY +* @n_rx_sc number of receive secure channels configured on this SecY +* @sci secure channel identifier used for tx +* @key_len length of keys used by the cipher suite +* @icv_len length of ICV used by the cipher suite +* @validate_frames validation mode +* @operational MAC_Operational flag +* @protect_frames enable protection for this SecY +* @replay_protect enable packet number checks on receive +* @replay_window size of the replay window +* @tx_sc transmit secure channel +* @rx_sc linked list of receive secure channels +*/ +struct macsec_secy { +struct net_device *netdev; +unsigned int n_rx_sc; +sci_t sci; +u16 key_len; +u16 icv_len; +enum validation_type validate_frames; +bool operational; +bool protect_frames; +bool replay_protect; +u32 replay_window; +struct macsec_tx_sc tx_sc; +struct macsec_rx_sc __rcu *rx_sc; +}; + +struct pcpu_secy_stats { +struct macsec_dev_stats stats; +struct u64_stats_sync syncp; +}; + +/** +* struct macsec_dev - private data +* @secy SecY config +* @real_dev pointer to underlying netdevice +* @stats MACsec device stats +* @secys linked list of SecY's on the underlying device +*/ +struct macsec_dev { +struct macsec_secy secy; +struct net_device *real_dev; +struct pcpu_secy_stats __percpu *stats; +struct list_head secys; +}; + +/** +* struct macsec_rxh_data - rx_handler private argument +* @secys linked list of SecY's on this underlying device +*/ +struct macsec_rxh_data { +struct list_head secys; +}; + +static struct macsec_dev *macsec_priv (const struct net_device *dev) +{ +return (struct macsec_dev *)netdev_priv (dev); +} + +static struct macsec_rxh_data *macsec_data_rcu (const struct net_device *dev) +{ +return rcu_dereference_bh (dev->rx_handler_data); +} + +static struct macsec_rxh_data *macsec_data_rtnl (const struct net_device *dev) +{ +return rtnl_dereference (dev->rx_handler_data); +} + +struct macsec_cb { +struct aead_request *req; +union { +struct macsec_tx_sa *tx_sa; +struct macsec_rx_sa *rx_sa; + }; +u8 assoc_num; +bool valid; +bool has_sci; +}; + +static struct macsec_rx_sa *macsec_rxsa_get (struct macsec_rx_sa __rcu *ptr) +{ +struct macsec_rx_sa *sa =rcu_dereference_bh (ptr); + +if (!sa || !sa->active) +return NULL ; + +if (!atomic_inc_not_zero (&sa->refcnt)) +return NULL ; + +return sa; +} + +static void free_rx_sc_rcu (struct rcu_head *head) +{ + struct macsec_rx_sc *rx_sc = container_of(head, struct macsec_rx_sc, rcu_head); + +free_percpu (rx_sc->stats); +kfree (rx_sc); +} + +static struct macsec_rx_sc *macsec_rxsc_get (struct macsec_rx_sc *sc) +{ +return atomic_inc_not_zero (&sc->refcnt) ? sc :NULL ; +} + +static void macsec_rxsc_put (struct macsec_rx_sc *sc) +{ +if (atomic_dec_and_test (&sc->refcnt)) +call_rcu (&sc->rcu_head, free_rx_sc_rcu); +} + +static void free_rxsa (struct rcu_head *head) +{ + struct macsec_rx_sa *sa = container_of(head, struct macsec_rx_sa, rcu); + +crypto_free_aead (sa->key.tfm); +free_percpu (sa->stats); +macsec_rxsc_put (sa->sc); +kfree (sa); +} + +static void macsec_rxsa_put (struct macsec_rx_sa *sa) +{ +if (atomic_dec_and_test (&sa->refcnt)) +call_rcu (&sa->rcu, free_rxsa); +} + +static struct macsec_tx_sa *macsec_txsa_get (struct macsec_tx_sa __rcu *ptr) +{ +struct macsec_tx_sa *sa =rcu_dereference_bh (ptr); + +if (!sa || !sa->active) +return NULL ; + +if (!atomic_inc_not_zero (&sa->refcnt)) +return NULL ; + +return sa; +} + +static void free_txsa (struct rcu_head *head) +{ + struct macsec_tx_sa *sa = container_of(head, struct macsec_tx_sa, rcu); + +crypto_free_aead (sa->key.tfm); +free_percpu (sa->stats); +kfree (sa); +} + +static void macsec_txsa_put (struct macsec_tx_sa *sa) +{ +if (atomic_dec_and_test (&sa->refcnt)) +call_rcu (&sa->rcu, free_txsa); +} + +static struct macsec_cb *macsec_skb_cb (struct sk_buff *skb) +{ +BUILD_BUG_ON (sizeof (struct macsec_cb ) >sizeof (skb->cb)); +return (struct macsec_cb *)skb->cb; +} + +#define MACSEC_PORT_ES (htons(0x0001)) +#define MACSEC_PORT_SCB (0x0000) +#define MACSEC_UNDEF_SCI ((__force sci_t)0xffffffffffffffffULL) + +#define DEFAULT_SAK_LEN 16 +#define DEFAULT_SEND_SCI true +#define DEFAULT_ENCRYPT false +#define DEFAULT_ENCODING_SA 0 + +static sci_t make_sci (u8 *addr,__be16 port) +{ +sci_t sci; + +memcpy (&sci, addr,ETH_ALEN ); +memcpy (((char *)&sci) +ETH_ALEN , &port,sizeof (port)); + +return sci; +} + +static sci_t macsec_frame_sci (struct macsec_eth_header *hdr,bool sci_present) +{ +sci_t sci; + +if (sci_present) +memcpy (&sci, hdr->secure_channel_id, +sizeof (hdr->secure_channel_id)); +else + sci =make_sci (hdr->eth.h_source,MACSEC_PORT_ES ); + +return sci; +} + +static unsigned int macsec_sectag_len (bool sci_present) +{ +return MACSEC_TAG_LEN + (sci_present ?MACSEC_SCI_LEN :0 ); +} + +static unsigned int macsec_hdr_len (bool sci_present) +{ +return macsec_sectag_len (sci_present) +ETH_HLEN ; +} + +static unsigned int macsec_extra_len (bool sci_present) +{ +return macsec_sectag_len (sci_present) +sizeof (__be16); +} + +/* Fill SecTAG according to IEEE 802.1AE-2006 10.5.3 */ +static void macsec_fill_sectag (struct macsec_eth_header *h, +const struct macsec_secy *secy,u32 pn) +{ +const struct macsec_tx_sc *tx_sc = &secy->tx_sc; + +memset (&h->tci_an,0 ,macsec_sectag_len (tx_sc->send_sci)); + h->eth.h_proto =htons (ETH_P_MACSEC ); + +if (tx_sc->send_sci || + (secy->n_rx_sc >1 && !tx_sc->end_station && !tx_sc->scb)) { + h->tci_an |=MACSEC_TCI_SC ; +memcpy (&h->secure_channel_id, &secy->sci, +sizeof (h->secure_channel_id)); + }else { +if (tx_sc->end_station) + h->tci_an |=MACSEC_TCI_ES ; +if (tx_sc->scb) + h->tci_an |=MACSEC_TCI_SCB ; + } + + h->packet_number =htonl (pn); + +/* with GCM, C/E clear for !encrypt, both set for encrypt */ +if (tx_sc->encrypt) + h->tci_an |=MACSEC_TCI_CONFID ; +else if (secy->icv_len !=DEFAULT_ICV_LEN ) + h->tci_an |=MACSEC_TCI_C ; + + h->tci_an |= tx_sc->encoding_sa; +} + +static void macsec_set_shortlen (struct macsec_eth_header *h,size_t data_len) +{ +if (data_len <MACSEC_SHORTLEN_THR ) + h->short_length = data_len; +} + +/* minimum secure data length deemed "not short", see IEEE 802.1AE-2006 9.7 */ +#define MIN_NON_SHORT_LEN 48 + +/* validate MACsec packet according to IEEE 802.1AE-2006 9.12 */ +static bool macsec_validate_skb (struct sk_buff *skb,u16 icv_len) +{ +struct macsec_eth_header *h = (struct macsec_eth_header *)skb->data; +int len = skb->len -2 *ETH_ALEN ; +int extra_len =macsec_extra_len (!!(h->tci_an &MACSEC_TCI_SC )) + icv_len; + +/* a) It comprises at least 17 octets */ +if (skb->len <=16 ) +return false; + +/* b) MACsec EtherType: already checked */ + +/* c) V bit is clear */ +if (h->tci_an &MACSEC_TCI_VERSION ) +return false; + +/* d) ES or SCB => !SC */ +if ((h->tci_an &MACSEC_TCI_ES || h->tci_an &MACSEC_TCI_SCB ) && + (h->tci_an &MACSEC_TCI_SC )) +return false; + +/* e) Bits 7 and 8 of octet 4 of the SecTAG are clear */ +if (h->unused) +return false; + +/* rx.pn != 0 (figure 10-5) */ +if (!h->packet_number) +return false; + +/* length check, f) g) h) i) */ +if (h->short_length) +return len == extra_len + h->short_length; +return len >= extra_len +MIN_NON_SHORT_LEN ; +} + +#define MACSEC_NEEDED_HEADROOM (macsec_extra_len(true)) +#define MACSEC_NEEDED_TAILROOM MACSEC_MAX_ICV_LEN + +static void macsec_fill_iv (unsigned char *iv,sci_t sci,u32 pn) +{ +struct gcm_iv *gcm_iv = (struct gcm_iv *)iv; + + gcm_iv->sci = sci; + gcm_iv->pn =htonl (pn); +} + +static struct macsec_eth_header *macsec_ethhdr (struct sk_buff *skb) +{ +return (struct macsec_eth_header *)skb_mac_header (skb); +} + +static u32 tx_sa_update_pn (struct macsec_tx_sa *tx_sa,struct macsec_secy *secy) +{ +u32 pn; + +spin_lock_bh (&tx_sa->lock); + pn = tx_sa->next_pn; + + tx_sa->next_pn++; +if (tx_sa->next_pn ==0 ) { +pr_debug ("PN wrapped, transitionning to !oper\n" ); + tx_sa->active = false; +if (secy->protect_frames) + secy->operational = false; + } +spin_unlock_bh (&tx_sa->lock); + +return pn; +} + +static void macsec_encrypt_finish (struct sk_buff *skb,struct net_device *dev) +{ +struct macsec_dev *macsec =netdev_priv (dev); + + skb->dev = macsec->real_dev; +skb_reset_mac_header (skb); + skb->protocol =eth_hdr (skb)->h_proto; +} + +static void macsec_count_tx (struct sk_buff *skb,struct macsec_tx_sc *tx_sc, +struct macsec_tx_sa *tx_sa) +{ +struct pcpu_tx_sc_stats *txsc_stats =this_cpu_ptr (tx_sc->stats); + +u64_stats_update_begin (&txsc_stats->syncp); +if (tx_sc->encrypt) { + txsc_stats->stats.OutOctetsEncrypted += skb->len; + txsc_stats->stats.OutPktsEncrypted++; +this_cpu_inc (tx_sa->stats->OutPktsEncrypted); + }else { + txsc_stats->stats.OutOctetsProtected += skb->len; + txsc_stats->stats.OutPktsProtected++; +this_cpu_inc (tx_sa->stats->OutPktsProtected); + } +u64_stats_update_end (&txsc_stats->syncp); +} + +static void count_tx (struct net_device *dev,int ret,int len) +{ +if (likely (ret ==NET_XMIT_SUCCESS || ret ==NET_XMIT_CN )) { +struct pcpu_sw_netstats *stats =this_cpu_ptr (dev->tstats); + +u64_stats_update_begin (&stats->syncp); + stats->tx_packets++; + stats->tx_bytes += len; +u64_stats_update_end (&stats->syncp); + }else { + dev->stats.tx_dropped++; + } +} + +static void macsec_encrypt_done (struct crypto_async_request *base,int err) +{ +struct sk_buff *skb = base->data; +struct net_device *dev = skb->dev; +struct macsec_dev *macsec =macsec_priv (dev); +struct macsec_tx_sa *sa =macsec_skb_cb (skb)->tx_sa; +int len, ret; + +aead_request_free (macsec_skb_cb (skb)->req); + +rcu_read_lock_bh (); +macsec_encrypt_finish (skb, dev); +macsec_count_tx (skb, &macsec->secy.tx_sc,macsec_skb_cb (skb)->tx_sa); + len = skb->len; + ret =dev_queue_xmit (skb); +count_tx (dev, ret, len); +rcu_read_unlock_bh (); + +macsec_txsa_put (sa); +dev_put (dev); +} + +static struct sk_buff *macsec_encrypt (struct sk_buff *skb, +struct net_device *dev) +{ +int ret; +struct scatterlist sg[MAX_SKB_FRAGS +1 ]; +unsigned char iv[GCM_AES_IV_LEN ]; +struct ethhdr *eth; +struct macsec_eth_header *hh; +size_t unprotected_len; +struct aead_request *req; +struct macsec_secy *secy; +struct macsec_tx_sc *tx_sc; +struct macsec_tx_sa *tx_sa; +struct macsec_dev *macsec =macsec_priv (dev); +u32 pn; + + secy = &macsec->secy; + tx_sc = &secy->tx_sc; + +/* 10.5.1 TX SA assignment */ + tx_sa =macsec_txsa_get (tx_sc->sa[tx_sc->encoding_sa]); +if (!tx_sa) { + secy->operational = false; +kfree_skb (skb); +return ERR_PTR (-EINVAL ); + } + +if (unlikely (skb_headroom (skb) <MACSEC_NEEDED_HEADROOM || +skb_tailroom (skb) <MACSEC_NEEDED_TAILROOM )) { +struct sk_buff *nskb =skb_copy_expand (skb, +MACSEC_NEEDED_HEADROOM , +MACSEC_NEEDED_TAILROOM , +GFP_ATOMIC ); +if (likely (nskb)) { +consume_skb (skb); + skb = nskb; + }else { +macsec_txsa_put (tx_sa); +kfree_skb (skb); +return ERR_PTR (-ENOMEM ); + } + }else { + skb =skb_unshare (skb,GFP_ATOMIC ); +if (!skb) { +macsec_txsa_put (tx_sa); +return ERR_PTR (-ENOMEM ); + } + } + + unprotected_len = skb->len; + eth =eth_hdr (skb); + hh = (struct macsec_eth_header *)skb_push (skb,macsec_extra_len (tx_sc->send_sci)); +memmove (hh, eth,2 *ETH_ALEN ); + + pn =tx_sa_update_pn (tx_sa, secy); +if (pn ==0 ) { +macsec_txsa_put (tx_sa); +kfree_skb (skb); +return ERR_PTR (-ENOLINK ); + } +macsec_fill_sectag (hh, secy, pn); +macsec_set_shortlen (hh, unprotected_len -2 *ETH_ALEN ); + +macsec_fill_iv (iv, secy->sci, pn); + +skb_put (skb, secy->icv_len); + +if (skb->len -ETH_HLEN >macsec_priv (dev)->real_dev->mtu) { +struct pcpu_secy_stats *secy_stats =this_cpu_ptr (macsec->stats); + +u64_stats_update_begin (&secy_stats->syncp); + secy_stats->stats.OutPktsTooLong++; +u64_stats_update_end (&secy_stats->syncp); + +macsec_txsa_put (tx_sa); +kfree_skb (skb); +return ERR_PTR (-EINVAL ); + } + + req =aead_request_alloc (tx_sa->key.tfm,GFP_ATOMIC ); +if (!req) { +macsec_txsa_put (tx_sa); +kfree_skb (skb); +return ERR_PTR (-ENOMEM ); + } + +sg_init_table (sg,MAX_SKB_FRAGS +1 ); +skb_to_sgvec (skb, sg,0 , skb->len); + +if (tx_sc->encrypt) { +int len = skb->len -macsec_hdr_len (tx_sc->send_sci) - + secy->icv_len; +aead_request_set_crypt (req, sg, sg, len, iv); +aead_request_set_ad (req,macsec_hdr_len (tx_sc->send_sci)); + }else { +aead_request_set_crypt (req, sg, sg,0 , iv); +aead_request_set_ad (req, skb->len - secy->icv_len); + } + +macsec_skb_cb (skb)->req = req; +macsec_skb_cb (skb)->tx_sa = tx_sa; +aead_request_set_callback (req,0 , macsec_encrypt_done, skb); + +dev_hold (skb->dev); + ret =crypto_aead_encrypt (req); +if (ret == -EINPROGRESS ) { +return ERR_PTR (ret); + }else if (ret !=0 ) { +dev_put (skb->dev); +kfree_skb (skb); +aead_request_free (req); +macsec_txsa_put (tx_sa); +return ERR_PTR (-EINVAL ); + } + +dev_put (skb->dev); +aead_request_free (req); +macsec_txsa_put (tx_sa); + +return skb; +} + +static bool macsec_post_decrypt (struct sk_buff *skb,struct macsec_secy *secy,u32 pn) +{ +struct macsec_rx_sa *rx_sa =macsec_skb_cb (skb)->rx_sa; +struct pcpu_rx_sc_stats *rxsc_stats =this_cpu_ptr (rx_sa->sc->stats); +struct macsec_eth_header *hdr =macsec_ethhdr (skb); +u32 lowest_pn =0 ; + +spin_lock (&rx_sa->lock); +if (rx_sa->next_pn >= secy->replay_window) + lowest_pn = rx_sa->next_pn - secy->replay_window; + +/* Now perform replay protection check again +* (see IEEE 802.1AE-2006 figure 10-5) +*/ +if (secy->replay_protect && pn < lowest_pn) { +spin_unlock (&rx_sa->lock); +u64_stats_update_begin (&rxsc_stats->syncp); + rxsc_stats->stats.InPktsLate++; +u64_stats_update_end (&rxsc_stats->syncp); +return false; + } + +if (secy->validate_frames !=MACSEC_VALIDATE_DISABLED ) { +u64_stats_update_begin (&rxsc_stats->syncp); +if (hdr->tci_an &MACSEC_TCI_E ) + rxsc_stats->stats.InOctetsDecrypted += skb->len; +else + rxsc_stats->stats.InOctetsValidated += skb->len; +u64_stats_update_end (&rxsc_stats->syncp); + } + +if (!macsec_skb_cb (skb)->valid) { +spin_unlock (&rx_sa->lock); + +/* 10.6.5 */ +if (hdr->tci_an &MACSEC_TCI_C || + secy->validate_frames ==MACSEC_VALIDATE_STRICT ) { +u64_stats_update_begin (&rxsc_stats->syncp); + rxsc_stats->stats.InPktsNotValid++; +u64_stats_update_end (&rxsc_stats->syncp); +return false; + } + +u64_stats_update_begin (&rxsc_stats->syncp); +if (secy->validate_frames ==MACSEC_VALIDATE_CHECK ) { + rxsc_stats->stats.InPktsInvalid++; +this_cpu_inc (rx_sa->stats->InPktsInvalid); + }else if (pn < lowest_pn) { + rxsc_stats->stats.InPktsDelayed++; + }else { + rxsc_stats->stats.InPktsUnchecked++; + } +u64_stats_update_end (&rxsc_stats->syncp); + }else { +u64_stats_update_begin (&rxsc_stats->syncp); +if (pn < lowest_pn) { + rxsc_stats->stats.InPktsDelayed++; + }else { + rxsc_stats->stats.InPktsOK++; +this_cpu_inc (rx_sa->stats->InPktsOK); + } +u64_stats_update_end (&rxsc_stats->syncp); + +if (pn >= rx_sa->next_pn) + rx_sa->next_pn = pn +1 ; +spin_unlock (&rx_sa->lock); + } + +return true; +} + +static void macsec_reset_skb (struct sk_buff *skb,struct net_device *dev) +{ + skb->pkt_type =PACKET_HOST ; + skb->protocol =eth_type_trans (skb, dev); + +skb_reset_network_header (skb); +if (!skb_transport_header_was_set (skb)) +skb_reset_transport_header (skb); +skb_reset_mac_len (skb); +} + +static void macsec_finalize_skb (struct sk_buff *skb,u8 icv_len,u8 hdr_len) +{ +memmove (skb->data + hdr_len, skb->data,2 *ETH_ALEN ); +skb_pull (skb, hdr_len); +pskb_trim_unique (skb, skb->len - icv_len); +} + +static void count_rx (struct net_device *dev,int len) +{ +struct pcpu_sw_netstats *stats =this_cpu_ptr (dev->tstats); + +u64_stats_update_begin (&stats->syncp); + stats->rx_packets++; + stats->rx_bytes += len; +u64_stats_update_end (&stats->syncp); +} + +static void macsec_decrypt_done (struct crypto_async_request *base,int err) +{ +struct sk_buff *skb = base->data; +struct net_device *dev = skb->dev; +struct macsec_dev *macsec =macsec_priv (dev); +struct macsec_rx_sa *rx_sa =macsec_skb_cb (skb)->rx_sa; +int len, ret; +u32 pn; + +aead_request_free (macsec_skb_cb (skb)->req); + +rcu_read_lock_bh (); + pn =ntohl (macsec_ethhdr (skb)->packet_number); +if (!macsec_post_decrypt (skb, &macsec->secy, pn)) { +rcu_read_unlock_bh (); +kfree_skb (skb); + goto out; + } + +macsec_finalize_skb (skb, macsec->secy.icv_len, +macsec_extra_len (macsec_skb_cb (skb)->has_sci)); +macsec_reset_skb (skb, macsec->secy.netdev); + + len = skb->len; + ret =netif_rx (skb); +if (ret ==NET_RX_SUCCESS ) +count_rx (dev, len); +else + macsec->secy.netdev->stats.rx_dropped++; + +rcu_read_unlock_bh (); + +out: +macsec_rxsa_put (rx_sa); +dev_put (dev); +return ; +} + +static struct sk_buff *macsec_decrypt(struct sk_buff *skb, + struct net_device *dev, + struct macsec_rx_sa *rx_sa, + sci_t sci,
drivers/net/macsec.c line 836
Quoted 10 lines
drivers/net/macsec.c new file +10
-0+ struct macsec_secy *secy) +{ +int ret; +struct scatterlist sg[MAX_SKB_FRAGS +1 ]; +unsigned char iv[GCM_AES_IV_LEN ]; +struct aead_request *req; +struct macsec_eth_header *hdr; +u16 icv_len = secy->icv_len; + +macsec_skb_cb (skb)->valid =0 ;
Please use true/false for boolean variables.
drivers/net/macsec.c line 890
Quoted 1 line
drivers/net/macsec.c new file +1
-0+macsec_skb_cb (skb)->valid =1 ;
Likewise.
drivers/net/macsec.c line 928
Quoted 6 lines
drivers/net/macsec.c new file +6
-0+static void handle_not_macsec (struct sk_buff *skb) +{ +struct macsec_rxh_data *rxd; +struct macsec_dev *macsec; + +rcu_read_lock_bh ();
"bh" should be implicit in this receive code path, so plain rcu_read_lock() should be sufficient.
Quoted 2 lines
drivers/net/macsec.c new file +2
-0+static rx_handler_result_t macsec_handle_frame (struct sk_buff **pskb) +{
...
drivers/net/macsec.c
Quoted 1 line
drivers/net/macsec.c new file +1
-0+rcu_read_lock_bh ();
Likewise.
Otherwise looks really good to me.
Re: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver
In reply to Sabrina Dubrocadrivers/net/macsec.c line 77
Quoted 8 lines · Sabrina Dubroca
drivers/net/macsec.c new file +7
-0+struct gcm_iv { +union { +u8 secure_channel_id[8 ]; +sci_t sci; + }; +__be32 pn; +};
Should this be __packed?
But the struct is confusing; sci_t is a host type (that depends on endianness), and __be32 would seem to be a network type, how can they both be in the same struct? Or does sci_t have to be __be64?
drivers/net/macsec.c line 100
Quoted 12 lines
drivers/net/macsec.c new file +12
-0+/** +* struct macsec_rx_sa - receive secure association +* @active +* @next_pn packet number expected for the next packet +* @lock protects next_pn manipulations +* @key key structure +* @stats per-SA stats +*/ +struct macsec_rx_sa { +bool active; +u32 next_pn; +spinlock_t lock;
If you put the spinlock first or at least next to active you can get rid of some padding (on arches/configs where spinlock is small, at least)
Quoted 7 lines
+/**
+ * structmacsec_rx_sc- receive secure channel
+ * @sci secure channel identifier for this SC
+ * @active channel is active
+ * @sa array of secure associations
+ * @stats per-SC stats
+ */
Btw, all your kernel-doc comments are actually malformed, they're missing a colon after the @member, e.g.
@stats: per-SC stats
drivers/net/macsec.c line 171
Quoted 8 lines
drivers/net/macsec.c new file +8
-0+struct macsec_tx_sc { +bool active; +u8 encoding_sa; +bool encrypt; +bool send_sci; +bool end_station; +bool scb; +struct macsec_tx_sa __rcu *sa[4 ];
What's 4?
drivers/net/macsec.c line 359
Quoted 9 lines
drivers/net/macsec.c new file +9
-0+static sci_t make_sci (u8 *addr,__be16 port) +{ +sci_t sci; + +memcpy (&sci, addr,ETH_ALEN ); +memcpy (((char *)&sci) +ETH_ALEN , &port,sizeof (port)); + +return sci; +}
Oh, maybe this explains my earlier question - looks like the sci_t isn't really a 64-bit integer but some kind of structure.
Is there really much point in using a __bitwise u64 typedef, rather than a small packed struct then?
Quoted 3 lines
+/* minimum secure data length deemed "not short", see IEEE 802.1AE- 2006 9.7 */
+#defineMIN_NON_SHORT_LEN48
I saw
Quoted 1 line
+#define
MACSEC_SHORTLEN_THR48
before, are they different? Seem very similar.
drivers/net/macsec.c line 491
Quoted 3 lines
drivers/net/macsec.c new file +3
-0+ tx_sa->next_pn++; +if (tx_sa->next_pn ==0 ) { +pr_debug ("PN wrapped, transitionning to !oper\n" );
typo: transitioning
drivers/net/macsec.c line 2224
Quoted 11 lines
drivers/net/macsec.c new file +11
-0+static const struct genl_ops macsec_genl_ops[] = { + { + .cmd =MACSEC_CMD_GET_TXSC , + .dumpit = macsec_dump_txsc, + .policy = macsec_genl_policy, + }, + { + .cmd =MACSEC_CMD_ADD_RXSC , + .doit = macsec_add_rxsc, + .policy = macsec_genl_rxsc_policy, + .flags =GENL_ADMIN_PERM ,
IMHO, having different policies for different operations is pretty confusing. I now slowly start to understand why you had to do all this aliasing with the IDs. However, perhaps it'd be better to define a top- level attribute list with the ifindex etc., and make all the *additional* data needed for RXSC operations for example go into a nested attribute in the top-level.
That way, you have the same policy for everything and also don't have to play tricks with the aliasing since the top-level attributes actually exist now, coming from the same namespace & policy.
johannes
Sabrina Dubroca
maintainerRe: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver
Quoted 11 lines · Johannes Berg
drivers/net/macsec.c line 77
Quoted 8 lines · Sabrina Dubroca
drivers/net/macsec.c new file +7
-0+struct gcm_iv { +union { +u8 secure_channel_id[8 ]; +sci_t sci; + }; +__be32 pn; +};Should this be
__packed?
I think that's not necessary here.
Quoted 4 lines
But the struct is confusing;
sci_tis a host type (that depends on endianness), and__be32would seem to be a network type, how can they both be in the same struct? Or doessci_thave to be__be64?
Quoted 16 lines
drivers/net/macsec.c line 100
Quoted 12 lines
drivers/net/macsec.c new file +12
-0+/** +* struct macsec_rx_sa - receive secure association +* @active +* @next_pn packet number expected for the next packet +* @lock protects next_pn manipulations +* @key key structure +* @stats per-SA stats +*/ +struct macsec_rx_sa { +bool active; +u32 next_pn; +spinlock_t lock;If you put the spinlock first or at least next to active you can get rid of some padding (on arches/configs where spinlock is small, at least)
Ok, I rearranged macsec_rx_sa and macsec_tx_sa.
Quoted 8 lines
Quoted 7 lines
+/**
+ * structmacsec_rx_sc- receive secure channel
+ * @sci secure channel identifier for this SC
+ * @active channel is active
+ * @sa array of secure associations
+ * @stats per-SC stats
+ */
Quoted 4 lines
Btw, all your kernel-doc comments are actually malformed, they're missing a colon after the @member, e.g.
@stats: per-SC stats
duh, I never noticed that :(
Thanks.
Quoted 11 lines
drivers/net/macsec.c line 171
Quoted 8 lines
drivers/net/macsec.c new file +8
-0+struct macsec_tx_sc { +bool active; +u8 encoding_sa; +bool encrypt; +bool send_sci; +bool end_station; +bool scb; +struct macsec_tx_sa __rcu *sa[4 ];What's 4?
Good point. I added:
#define MACSEC_NUM_AN 4 /* 2 bits for the association number */
and used it in all the range tests (< 4, >= 3).
Quoted 4 lines
Quoted 4 lines
drivers/net/macsec.c new file +4
-0+static sci_t make_sci (u8 *addr,__be16 port) +{ +sci_t sci; +
Quoted 8 lines
drivers/net/macsec.c line 359
Quoted 5 lines
drivers/net/macsec.c new file +5
-0+memcpy (&sci, addr,ETH_ALEN ); +memcpy (((char *)&sci) +ETH_ALEN , &port,sizeof (port)); + +return sci; +}Oh, maybe this explains my earlier question - looks like the
sci_tisn't really a 64-bit integer but some kind of structure.
Yes, the bits in the SCI have some meaning.
Quoted 2 lines
Is there really much point in using a
__bitwiseu64 typedef, rather than a small packed struct then?
I can use == tests, as in find_rx_sc().
Quoted 9 lines
Quoted 3 lines
+/* minimum secure data length deemed "not short", see IEEE 802.1AE- 2006 9.7 */
+#defineMIN_NON_SHORT_LEN48I saw
Quoted 1 line
+#define
MACSEC_SHORTLEN_THR48before, are they different? Seem very similar.
They are exactly the same, good catch.
Quoted 5 lines
drivers/net/macsec.c line 491
Quoted 3 lines
drivers/net/macsec.c new file +3
-0+ tx_sa->next_pn++; +if (tx_sa->next_pn ==0 ) { +pr_debug ("PN wrapped, transitionning to !oper\n" );typo: transitioning
Fixed.
Quoted 19 lines
drivers/net/macsec.c line 2224
Quoted 11 lines
drivers/net/macsec.c new file +11
-0+static const struct genl_ops macsec_genl_ops[] = { + { + .cmd =MACSEC_CMD_GET_TXSC , + .dumpit = macsec_dump_txsc, + .policy = macsec_genl_policy, + }, + { + .cmd =MACSEC_CMD_ADD_RXSC , + .doit = macsec_add_rxsc, + .policy = macsec_genl_rxsc_policy, + .flags =GENL_ADMIN_PERM ,IMHO, having different policies for different operations is pretty confusing. I now slowly start to understand why you had to do all this aliasing with the IDs. However, perhaps it'd be better to define a top- level attribute list with the ifindex etc., and make all the *additional* data needed for RXSC operations for example go into a nested attribute in the top-level.
Quoted 3 lines
That way, you have the same policy for everything and also don't have to play tricks with the aliasing since the top-level attributes actually exist now, coming from the same namespace & policy.
That's a good idea, I'll have a look today.
I don't really like the aliasing games I have to play, but I'd like to keep the RXSC attributes separate from the SA attributes, I think it looks cleaner (the first RFC had everything in the same policy:
http://www.spinics.net/lists/netdev/msg358152.html).
Thanks for the review.
Signature · 1 line
Sabrina
Re: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver
Hi,
Thanks for the comments.
Quoted 11 lines
Quoted 9 lines
drivers/net/macsec.c line 77
Quoted 7 lines
drivers/net/macsec.c new file +7
-0+struct gcm_iv { +union { +u8 secure_channel_id[8 ]; +sci_t sci; + }; +__be32 pn; +};Should this be
__packed?I think that's not necessary here.
Yeah, there's probably no way a compiler could ever do something with it that's not the same as packed, but it seems to me that just out of convention structs that have some wire-format meaning should usually be __packed. But it's your call. I'm not even entirely sure it has a wire- format or similar meaning, although the name indicates it gets used into the encryption and must be exactly these 12 octets.
Quoted 5 lines
Quoted 4 lines
That way, you have the same policy for everything and also don't have
to play tricks with the aliasing since the top-level attributes actually exist now, coming from the same namespace & policy.
Quoted 7 lines
That's a good idea, I'll have a look today.
I don't really like the aliasing games I have to play, but I'd like to
keep the RXSC attributes separate from the SA attributes, I think it looks cleaner (the first RFC had everything in the same policy:
http://www.spinics.net/lists/netdev/msg358152.html).
Ah, I see. Keeping it separate makes sense, but you can still achieve that like this:
msg = [ifindex -> 7,
rxsc -> [ sci -> 2, ...
],
...
]
with nested data. I'd also do the same for the stats and a whole bunch of others too, I guess. I tend to imagine it as a kind of "dict of dicts" (the message with nested).
Thanks,
johannes
David Miller
maintainerRe: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver
From: Johannes Berg <[email protected]>
Date: Wed, 09 Mar 2016 12:24:20 +0100
Quoted 4 lines
Yeah, there's probably no way a compiler could ever do something with it that's not the same as packed, but it seems to me that just out of convention structs that have some wire-format meaning should usually be
__packed.
Not unless absolutely necessary.
There is a serious performance penalty for using __packed on some architectures. __packed has several undesirable side-effects, one of which is that it causes the compiler to be unable to assume the alignment of anything.
Sabrina Dubroca
maintainerPATCH 1/1 ip: add MACsec support
In reply to Sabrina DubrocaExtend ip-link to create MACsec devices
ip link add link <master> <macsec> type macsec [options]
Add ip macsec command to configure receive-side secure channels and secure associations within a macsec netdevice.
---
This needs the MACsec UAPI headers.
ip/Makefile | 2 +- ip/ip.c | 3 +- ip/ip_common.h | 1 + ip/ipmacsec.c | 1140 +++++++++++++++++++++++++++++++++++++++++++++++++ man/man8/Makefile | 2 +- man/man8/ip-link.8.in | 134 ++++++ man/man8/ip-macsec.8 | 110 +++++ 7 files changed, 1389 insertions(+), 3 deletions(-) create mode 100644 ip/ipmacsec.c create mode 100644 man/man8/ip-macsec.8
ip/Makefile +1 -1
@@ -7,7 +7,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \ iplink_vxlan.o tcp_metrics.o iplink_ipoib.o ipnetconf.o link_ip6tnl.o \ link_iptnl.o link_gre6.o iplink_bond.o iplink_bond_slave.o iplink_hsr.o \ iplink_bridge.o iplink_bridge_slave.o ipfou.o iplink_ipvlan.o \ - iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o + iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o RTMONOBJ=rtmon.o
ip/ip.c +2 -1
@@ -51,7 +51,7 @@ static void usage(void)" ip [ -force ] -batch filename\n" "where OBJECT := { link | address | addrlabel | route | rule | neighbor | ntable |\n" " tunnel | tuntap | maddress | mroute | mrule | monitor | xfrm |\n" -" netns | l2tp | fou | tcp_metrics | token | netconf }\n" +" netns | l2tp | fou | macsec | tcp_metrics | token | netconf }\n" " OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n" " -h[uman-readable] | -iec |\n" " -f[amily] { inet | inet6 | ipx | dnet | mpls | bridge | link } |\n"
@@ -84,6 +84,7 @@ static const struct cmd { { "link", do_iplink }, { "l2tp", do_ipl2tp }, { "fou", do_ipfou }, + { "macsec", do_ipmacsec }, { "tunnel", do_iptunnel }, { "tunl", do_iptunnel }, { "tuntap", do_iptuntap },
ip/ip_common.h +1 -0
@@ -43,6 +43,7 @@ int do_iptunnel(int argc, char **argv);int do_ip6tunnel (int argc,char **argv);int do_iptuntap (int argc,char **argv);int do_iplink (int argc,char **argv); +int do_ipmacsec (int argc,char **argv);int do_ipmonitor (int argc,char **argv);int do_multiaddr (int argc,char **argv);int do_multiroute (int argc,char **argv);
ip/ipmacsec.c new file +1140 -0
1,140 lines not on this page · raw message
man/man8/Makefile +1 -1
@@ -7,7 +7,7 @@ MAN8PAGES = $(TARGETS) ip.8 arpd.8 lnstat.8 routel.8 rtacct.8 rtmon.8 rtpr.8 ss. tc-mqprio.8 tc-netem.8 tc-pfifo.8 tc-pfifo_fast.8 tc-prio.8 tc-red.8 \ tc-sfb.8 tc-sfq.8 tc-stab.8 tc-tbf.8 \ bridge.8 rtstat.8 ctstat.8 nstat.8 routef.8 \ - ip-addrlabel.8 ip-fou.8 ip-gue.8 ip-l2tp.8 \ + ip-addrlabel.8 ip-fou.8 ip-gue.8 ip-l2tp.8 ip-macsec.8 \ ip-maddress.8 ip-monitor.8 ip-mroute.8 ip-neighbour.8 \ ip-netns.8 ip-ntable.8 ip-rule.8 ip-tunnel.8 ip-xfrm.8 \ ip-tcp_metrics.8 ip-netconf.8 ip-token.8 \
man/man8/ip-link.8.in +134 -0
146 lines not on this page · raw message
man/man8/ip-macsec.8 new file +110 -0
110 lines not on this page · raw message
Signature · 1 line
2.7.2
Re: [PATCH iproute2 net-next] ip: add MACsec support
Quoted 39 lines
ip/ipmacsec.c new file +38
-0+static int one_of (const char *msg,const char *realval,const char **list, +size_t len,int *index) +{ +int i; + +for (i =0 ; i < len; i++) { +if (matches (realval, list[i]) ==0 ) { + *index = i; +return 0 ; + } + } + +fprintf (stderr,"Error: argument of \"%s\" must be one of " , msg); +for (i =0 ; i < len; i++) +fprintf (stderr,"\"%s\", " , list[i]); +fprintf (stderr,"not \"%s\"\n" , realval); +return -1 ; +} + +static int get_an (__u8 *val,const char *arg) +{ +int ret =get_u8 (val, arg,0 ); + +if (ret) +return ret; + +if (*val >3 ) +return -1 ; + +return 0 ; +} + +static int get_sci (__u64 *sci,const char *arg) +{ +return get_u64 (sci, arg,16 ); +} + +static int get_port (__be16 *port,const char *arg)
ip/ipmacsec.c line 165
Quoted 10 lines
ip/ipmacsec.c new file +10
-0+{ +__u16 p; +int ret =get_u16 (&p, arg,10 ); + +if (ret) +return ret; + + *port =htons (p); +return 0 ; +}
Some of these should probably go into lib
ip/ipmacsec.c line 177
Quoted 11 lines
ip/ipmacsec.c new file +11
-0+static int from_hex (char c) +{ +if (c >='0' && c <='9' ) +return c -'0' ; +if (c >='a' && c <='f' ) +return c -'a' +10 ; +if (c >='A' && c <='F' ) +return c -'A' +10 ; + +return -1 ; +}
There already several copies of similar code (ipl2tp, ipmaddr, ipx) for basically the same code.
...
Quoted 11 lines
ip/ipmacsec.c new file +11
-0+print_flag (stdout, attrs,"sc" ,MACSEC_ATTR_SC_ACTIVE ); +print_flag (stdout, attrs,"sa" ,MACSEC_ATTR_SA_ACTIVE ); +print_flag (stdout, attrs,"encrypt" ,MACSEC_ATTR_ENCRYPT ); +print_flag (stdout, attrs,"send_sci" ,MACSEC_ATTR_INC_SCI ); +print_flag (stdout, attrs,"end_station" ,MACSEC_ATTR_ES ); +print_flag (stdout, attrs,"scb" ,MACSEC_ATTR_SCB ); + +print_flag (stdout, attrs,"replayprotect" ,MACSEC_ATTR_REPLAY ); +if (attrs[MACSEC_ATTR_WINDOW ]) { +printf ("window %d " , +rta_getattr_u32 (attrs[MACSEC_ATTR_WINDOW ]));
ip/ipmacsec.c line 644
Quoted 10 lines
ip/ipmacsec.c new file +10
-0+ } + +if (attrs[MACSEC_ATTR_CIPHER_SUITE ] && attrs[MACSEC_ATTR_ICV_LEN ]) { +printf ("\n" ); +print_cipher_suite (prefix, +rta_getattr_u64 (attrs[MACSEC_ATTR_CIPHER_SUITE ]), +rta_getattr_u8 (attrs[MACSEC_ATTR_ICV_LEN ])); + } + +}
It is unwritten rule of ip commands that the print and set commands should be invertable. I.e if you are going to print an attribute the format should be the same as the argument passed in.
Sabrina Dubroca
maintainerRe: [PATCH iproute2 net-next] ip: add MACsec support
Quoted 43 lines · Stephen Hemminger
Quoted 43 lines
ip/ipmacsec.c new file +42
-0+static int one_of (const char *msg,const char *realval,const char **list, +size_t len,int *index) +{ +int i; + +for (i =0 ; i < len; i++) { +if (matches (realval, list[i]) ==0 ) { + *index = i; +return 0 ; + } + } + +fprintf (stderr,"Error: argument of \"%s\" must be one of " , msg); +for (i =0 ; i < len; i++) +fprintf (stderr,"\"%s\", " , list[i]); +fprintf (stderr,"not \"%s\"\n" , realval); +return -1 ; +} + +static int get_an (__u8 *val,const char *arg) +{ +int ret =get_u8 (val, arg,0 ); + +if (ret) +return ret; + +if (*val >3 ) +return -1 ; + +return 0 ; +} + +static int get_sci (__u64 *sci,const char *arg) +{ +return get_u64 (sci, arg,16 ); +} + +static int get_port (__be16 *port,const char *arg) +{ +__u16 p; +int ret =get_u16 (&p, arg,10 ); +
Quoted 8 lines
ip/ipmacsec.c line 165
Quoted 6 lines
ip/ipmacsec.c new file +6
-0+if (ret) +return ret; + + *port =htons (p); +return 0 ; +}Some of these should probably go into lib
I could add get_be16:
int get_be16(__be16 *val, const char *arg, int base);
And maybe get_u8_range for get_an:
int get_u8_range(__u8 *val, const char *arg, int base, int start, int end);
I could also move one_of to lib, not sure if that would be used. There are a lot of on/off in iplink, but the way I wrote one_of doesn't really work to set/clear a flag.
Adding a function pointer to handle a match seems a bit heavy.
What do you think?
Quoted 6 lines
Quoted 6 lines
ip/ipmacsec.c new file +6
-0+static int from_hex (char c) +{ +if (c >='0' && c <='9' ) +return c -'0' ; +if (c >='a' && c <='f' ) +return c -'a' +10 ;
Quoted 8 lines
ip/ipmacsec.c line 177
Quoted 5 lines
ip/ipmacsec.c new file +5
-0+if (c >='A' && c <='F' ) +return c -'A' +10 ; + +return -1 ; +}There already several copies of similar code (ipl2tp, ipmaddr, ipx) for basically the same code.
Right, I'll make a common helper for that.
I also noticed hexstring_a2n, which I could use if it also returned the number of elements it parsed to the caller, so I will make that change (it has no user for now).
Quoted 18 lines
...
Quoted 17 lines
ip/ipmacsec.c new file +17
-0+print_flag (stdout, attrs,"sc" ,MACSEC_ATTR_SC_ACTIVE ); +print_flag (stdout, attrs,"sa" ,MACSEC_ATTR_SA_ACTIVE ); +print_flag (stdout, attrs,"encrypt" ,MACSEC_ATTR_ENCRYPT ); +print_flag (stdout, attrs,"send_sci" ,MACSEC_ATTR_INC_SCI ); +print_flag (stdout, attrs,"end_station" ,MACSEC_ATTR_ES ); +print_flag (stdout, attrs,"scb" ,MACSEC_ATTR_SCB ); + +print_flag (stdout, attrs,"replayprotect" ,MACSEC_ATTR_REPLAY ); +if (attrs[MACSEC_ATTR_WINDOW ]) { +printf ("window %d " , +rta_getattr_u32 (attrs[MACSEC_ATTR_WINDOW ])); + } + +if (attrs[MACSEC_ATTR_CIPHER_SUITE ] && attrs[MACSEC_ATTR_ICV_LEN ]) { +printf ("\n" ); + print_cipher_suite(prefix, + rta_getattr_u64(attrs[MACSEC_ATTR_CIPHER_SUITE]),
Quoted 8 lines
ip/ipmacsec.c line 644
Quoted 4 lines
ip/ipmacsec.c new file +4
-0+ rta_getattr_u8(attrs[MACSEC_ATTR_ICV_LEN])); + } + +}It is unwritten rule of ip commands that the print and set commands should be invertable. I.e if you are going to print an attribute the format should be the same as the argument passed in.
Oops, will fix.
Thanks,
Signature · 1 line
Sabrina