[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 0/9] ixgbe: Add ipsec offload

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 0/9] ixgbe: Add ipsec offload

[Shannon Nelson]

This is a first working draft for adding IPsec offload processing support
to the ixgbe driver for the Niantic family of Intel NICs.  I'm posting
this in order to get some review comments from the Intel folks and from
anyone else interested in the XFRM/IPsec offloads.  These patches apply
to net-next 4.14-rc4.

This code is only slightly broken in that
 - it doesn't handle Tx checksums quite correctly, so it really needs to
   be disabled with "ethtool -K ethN tx off" for now
 - it doesn't even try to handle TSO correctly yet
 - it doesn't handle IPv6 traffic yet

Please feel free to post or direct email comments.  Meanwhile, I'll work
on cleaning up the remaining issues.

As is, this code does show improvements in both packet throughput
and CPU utilization.  For example, here are some quicky numbers with
single run of "iperf -c <dest>" with the ipsec offload on both ends of
a point-to-point connection, measured on a Xeon 5520:

	9.4 Gbps - normal case
	7.4 Gbps - tx csum off
	6.6 Gbps - ipsec offload on and tx csum off  (0.06 %system)
	344 Mbps - ipsec no offload and tx csum off  (6.31 %system)
	343 Mbps - ipsec no offload and tx csum on 

To set up a test case, you first need to be sure you have a recent version
of iproute2 that supports the ipsec offload tag, probably something from
ip 4.12 or newer would b e bets, tho' I think 4.9 will work.  I have a
shell script that builds up the appropriate commands for me, but here are
the resulting commands for all tcp traffic between 14.0.0.52 and 14.0.0.70:

For the left side (14.0.0.52):
  ip x p add dir out src 14.0.0.52/24 dst 14.0.0.70/24 proto tcp tmpl \
     proto esp src 14.0.0.52 dst 14.0.0.70 spi 0x07 mode transport reqid 0x07
  ip x p add dir in src 14.0.0.70/24 dst 14.0.0.52/24 proto tcp tmpl \
     proto esp dst 14.0.0.52 src 14.0.0.70 spi 0x07 mode transport reqid 0x07
  ip x s add proto esp src 14.0.0.52 dst 14.0.0.70 spi 0x07 mode transport \
     reqid 0x07 replay-window 32 \
     aead 'rfc4106(gcm(aes))' 0x44434241343332312423222114131211f4f3f2f1 128 \
     sel src 14.0.0.52/24 dst 14.0.0.70/24 proto tcp offload dev eth4 dir out
  ip x s add proto esp dst 14.0.0.52 src 14.0.0.70 spi 0x07 mode transport \
     reqid 0x07 replay-window 32 \
     aead 'rfc4106(gcm(aes))' 0x44434241343332312423222114131211f4f3f2f1 128 \
     sel src 14.0.0.70/24 dst 14.0.0.52/24 proto tcp offload dev eth4 dir in
 
For the right side (14.0.0.70):
  ip x p add dir out src 14.0.0.70/24 dst 14.0.0.52/24 proto tcp tmpl \
     proto esp src 14.0.0.70 dst 14.0.0.52 spi 0x07 mode transport reqid 0x07
  ip x p add dir in src 14.0.0.52/24 dst 14.0.0.70/24 proto tcp tmpl \
     proto esp dst 14.0.0.70 src 14.0.0.52 spi 0x07 mode transport reqid 0x07
  ip x s add proto esp src 14.0.0.70 dst 14.0.0.52 spi 0x07 mode transport \
     reqid 0x07 replay-window 32 \
     aead 'rfc4106(gcm(aes))' 0x44434241343332312423222114131211f4f3f2f1 128 \
     sel src 14.0.0.70/24 dst 14.0.0.52/24 proto tcp offload dev eth4 dir out
  ip x s add proto esp dst 14.0.0.70 src 14.0.0.52 spi 0x07 mode transport \
     reqid 0x07 replay-window 32 \
     aead 'rfc4106(gcm(aes))' 0x44434241343332312423222114131211f4f3f2f1 128 \
     sel src 14.0.0.52/24 dst 14.0.0.70/24 proto tcp offload dev eth4 dir in

In both cases, the command "ip x s flush ; ip x p flush" will clean
it all out and remove the offloads.



Shannon Nelson (9):
  ixgbe: remove undefined and conflicting bits
  ixgbe: clean up ipsec defines
  ixgbe: add the ipsec support code files
  ixgbe: add ipsec data to driver struct
  ixgbe: add ipsec offload stats
  ixgbe: add ipsec debug helpers
  ixgbe: add tx ipsec offload
  ixgbe: add the Rx ipsec offload processing
  ixgbe: add init and remove ipsec support

 drivers/net/ethernet/intel/ixgbe/Makefile        |    1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h         |   43 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c |   16 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c |   31 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c   | 1133 ++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h   |   80 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    |   59 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h    |   27 +-
 8 files changed, 1349 insertions(+), 41 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
 create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h

-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 1/9] ixgbe: remove undefined and conflicting bits

[Shannon Nelson]

The PE and OSE error bits from the Advanced Receive Writeback
descriptor are undefined in the published Niantic family datasheets
and conflict with the defined SECERR bits, which we will need in
a coming patch.

The CE and LE error bits are also marked as reserved in the
datasheet, but I'll let someone else remove them.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index ffa0ee5..0a77941 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -2375,8 +2375,6 @@ enum {
 #define IXGBE_RXDADV_ERR_HBO    0x00800000 /*Header Buffer Overflow */
 #define IXGBE_RXDADV_ERR_CE     0x01000000 /* CRC Error */
 #define IXGBE_RXDADV_ERR_LE     0x02000000 /* Length Error */
-#define IXGBE_RXDADV_ERR_PE     0x08000000 /* Packet Error */
-#define IXGBE_RXDADV_ERR_OSE    0x10000000 /* Oversize Error */
 #define IXGBE_RXDADV_ERR_USE    0x20000000 /* Undersize Error */
 #define IXGBE_RXDADV_ERR_TCPE   0x40000000 /* TCP/UDP Checksum Error */
 #define IXGBE_RXDADV_ERR_IPE    0x80000000 /* IP Checksum Error */
@@ -2482,8 +2480,6 @@ enum {
 #define IXGBE_RXDADV_ERR_FRAME_ERR_MASK ( \
 				      IXGBE_RXDADV_ERR_CE | \
 				      IXGBE_RXDADV_ERR_LE | \
-				      IXGBE_RXDADV_ERR_PE | \
-				      IXGBE_RXDADV_ERR_OSE | \
 				      IXGBE_RXDADV_ERR_USE)
 
 /* Multicast bit mask */
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 2/9] ixgbe: clean up ipsec defines

[Shannon Nelson]

Clean up the ipsec/macsec descriptor bit definitions to match the rest
of the defines and file organization.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 0a77941..befed8b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -2321,11 +2321,6 @@ enum {
 #define IXGBE_TXD_CMD_VLE    0x40000000 /* Add VLAN tag */
 #define IXGBE_TXD_STAT_DD    0x00000001 /* Descriptor Done */
 
-#define IXGBE_RXDADV_IPSEC_STATUS_SECP                  0x00020000
-#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_PROTOCOL       0x08000000
-#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_LENGTH         0x10000000
-#define IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED            0x18000000
-#define IXGBE_RXDADV_IPSEC_ERROR_BIT_MASK               0x18000000
 /* Multiple Transmit Queue Command Register */
 #define IXGBE_MTQC_RT_ENA       0x1 /* DCB Enable */
 #define IXGBE_MTQC_VT_ENA       0x2 /* VMDQ2 Enable */
@@ -2375,6 +2370,14 @@ enum {
 #define IXGBE_RXDADV_ERR_HBO    0x00800000 /*Header Buffer Overflow */
 #define IXGBE_RXDADV_ERR_CE     0x01000000 /* CRC Error */
 #define IXGBE_RXDADV_ERR_LE     0x02000000 /* Length Error */
+
+#define IXGBE_RXDADV_ERR_MACSEC_NO_SA_MATCH  0x08000000
+#define IXGBE_RXDADV_ERR_MACSEC_REPLAY_ERROR 0x10000000
+#define IXGBE_RXDADV_ERR_MACSEC_BAD_SIG      0x18000000
+#define IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL  0x08000000
+#define IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH    0x10000000
+#define IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED   0x18000000
+
 #define IXGBE_RXDADV_ERR_USE    0x20000000 /* Undersize Error */
 #define IXGBE_RXDADV_ERR_TCPE   0x40000000 /* TCP/UDP Checksum Error */
 #define IXGBE_RXDADV_ERR_IPE    0x80000000 /* IP Checksum Error */
@@ -2396,6 +2399,7 @@ enum {
 #define IXGBE_RXDADV_STAT_FCSTAT_FCPRSP 0x00000020 /* 10: Recv. FCP_RSP */
 #define IXGBE_RXDADV_STAT_FCSTAT_DDP    0x00000030 /* 11: Ctxt w/ DDP */
 #define IXGBE_RXDADV_STAT_TS		0x00010000 /* IEEE 1588 Time Stamp */
+#define IXGBE_RXDADV_STAT_SECP          0x00020000 /* IPsec/MACsec pkt found */
 
 /* PSRTYPE bit definitions */
 #define IXGBE_PSRTYPE_TCPHDR    0x00000010
@@ -2462,13 +2466,6 @@ enum {
 #define IXGBE_RXDADV_PKTTYPE_ETQF_MASK  0x00000070 /* ETQF has 8 indices */
 #define IXGBE_RXDADV_PKTTYPE_ETQF_SHIFT 4          /* Right-shift 4 bits */
 
-/* Security Processing bit Indication */
-#define IXGBE_RXDADV_LNKSEC_STATUS_SECP         0x00020000
-#define IXGBE_RXDADV_LNKSEC_ERROR_NO_SA_MATCH   0x08000000
-#define IXGBE_RXDADV_LNKSEC_ERROR_REPLAY_ERROR  0x10000000
-#define IXGBE_RXDADV_LNKSEC_ERROR_BIT_MASK      0x18000000
-#define IXGBE_RXDADV_LNKSEC_ERROR_BAD_SIG       0x18000000
-
 /* Masks to determine if packets should be dropped due to frame errors */
 #define IXGBE_RXD_ERR_FRAME_ERR_MASK ( \
 				      IXGBE_RXD_ERR_CE | \
@@ -2889,6 +2886,7 @@ struct ixgbe_adv_tx_context_desc {
 				 IXGBE_ADVTXD_POPTS_SHIFT)
 #define IXGBE_ADVTXD_POPTS_TXSM (IXGBE_TXD_POPTS_TXSM << \
 				 IXGBE_ADVTXD_POPTS_SHIFT)
+#define IXGBE_ADVTXD_POPTS_IPSEC     0x00000400 /* IPSec offload request */
 #define IXGBE_ADVTXD_POPTS_ISCO_1ST  0x00000000 /* 1st TSO of iSCSI PDU */
 #define IXGBE_ADVTXD_POPTS_ISCO_MDL  0x00000800 /* Middle TSO of iSCSI PDU */
 #define IXGBE_ADVTXD_POPTS_ISCO_LAST 0x00001000 /* Last TSO of iSCSI PDU */
@@ -2904,7 +2902,6 @@ struct ixgbe_adv_tx_context_desc {
 #define IXGBE_ADVTXD_TUCMD_L4T_SCTP  0x00001000  /* L4 Packet TYPE of SCTP */
 #define IXGBE_ADVTXD_TUCMD_L4T_RSV     0x00001800 /* RSV L4 Packet TYPE */
 #define IXGBE_ADVTXD_TUCMD_MKRREQ    0x00002000 /*Req requires Markers and CRC*/
-#define IXGBE_ADVTXD_POPTS_IPSEC      0x00000400 /* IPSec offload request */
 #define IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP 0x00002000 /* IPSec Type ESP */
 #define IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN 0x00004000/* ESP Encrypt Enable */
 #define IXGBE_ADVTXT_TUCMD_FCOE      0x00008000       /* FCoE Frame Type */
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 3/9] ixgbe: add the ipsec support code files

[Shannon Nelson]

This adds the ipsec support code into the driver, but doesn't yet
include it into the compile.

It is possible that the .h file contents could simply go into
the ixgbe_type.h and ixgbe.h files.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 1133 ++++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h |   80 ++
 2 files changed, 1213 insertions(+)
 create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
 create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
new file mode 100644
index 0000000..177b915
--- /dev/null
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -0,0 +1,1133 @@
+/*******************************************************************************
+ *
+ * Intel 10 Gigabit PCI Express Linux driver
+ * Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * Contact Information:
+ * Linux NICS <linux.nics@intel.com>
+ * e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
+ * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+ *
+ ******************************************************************************/
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/netdevice.h>
+#include <linux/vmalloc.h>
+#include <linux/string.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+#include <linux/ipv6.h>
+#include <net/xfrm.h>
+#include <crypto/aead.h>
+
+#include "ixgbe.h"
+#include "ixgbe_type.h"
+#include "ixgbe_ipsec.h"
+
+/**
+ * ixgbe_ipsec_dump_info - dump in-memory SA tables for debugging
+ * @adapter: pointer to adapter structure
+ **/
+void ixgbe_ipsec_dump_info(struct ixgbe_adapter *adapter)
+{
+	struct net_device *dev = adapter->netdev;
+	int i;
+
+	netdev_info(dev, "IXGBE_IPSEC_INFO_DUMP\n");
+
+	netdev_info(dev, "  Tx SA table\n");
+	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
+		struct tx_sa *tsa = &adapter->tx_sa_tbl[i];
+
+		if (!tsa->used)
+			continue;
+
+		netdev_info(dev, " tx_sa_tbl[%i] key = 0x%08x %08x %08x %08x\n",
+			    i, tsa->key[0], tsa->key[1],
+			    tsa->key[2], tsa->key[3]);
+		netdev_info(dev, "              salt = 0x%08x encrypt = %d\n",
+			    tsa->salt, tsa->encrypt);
+	}
+
+	netdev_info(dev, "  Rx SA table\n");
+	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
+		struct rx_sa *rsa = &adapter->rx_sa_tbl[i];
+
+		if (!rsa->used)
+			continue;
+
+		netdev_info(dev, "rx_sa_tbl[%i] ipaddr = 0x%08x %08x %08x %08x\n",
+			    i, rsa->ipaddr[0], rsa->ipaddr[1],
+			    rsa->ipaddr[2], rsa->ipaddr[3]);
+		netdev_info(dev, "             spi = 0x%08x proto = 0x%x iptbl_ind = 0x%08x mode = 0x%04x\n",
+			    rsa->xs->id.spi, rsa->xs->id.proto,
+			    rsa->iptbl_ind, rsa->mode);
+		netdev_info(dev, "             key = 0x%08x %08x %08x %08x\n",
+			    rsa->key[0], rsa->key[1],
+			    rsa->key[2], rsa->key[3]);
+		netdev_info(dev, "             salt = 0x%08x decrypt = %d xs = %p\n",
+			    rsa->salt, rsa->decrypt, rsa->xs);
+	}
+
+	netdev_info(dev, "  Rx IP table\n");
+	for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
+		struct rx_ip_sa *ipsa = &adapter->rx_ip_sa_tbl[i];
+
+		if (!ipsa->used)
+			continue;
+
+		netdev_info(dev, "rx_ip_tbl[%i] ipaddr = 0x%08x %08x %08x %08x ref_cnt = 0x%08x\n",
+			    i, ipsa->ipaddr[0], ipsa->ipaddr[1],
+			    ipsa->ipaddr[2], ipsa->ipaddr[3],
+			    ipsa->ref_cnt);
+	}
+
+	netdev_info(dev, "  Offload counts\n");
+	for (i = 0; i < adapter->num_rx_queues; i++) {
+		struct ixgbe_ring *rx_ring = adapter->rx_ring[i];
+
+		if (rx_ring->rx_stats.ipsec_offloads ||
+		    rx_ring->rx_stats.ipsec_offload_faileds) {
+			netdev_info(dev, "rx_ring[%i] ipsec_offloads = %llu ipsec_offload_faileds = %llu\n",
+				    i, rx_ring->rx_stats.ipsec_offloads,
+				    rx_ring->rx_stats.ipsec_offload_faileds);
+		}
+	}
+}
+
+/**
+ * ixgbe_ipsec_dump_hw - dump the contents of the hw tables
+ * @adapter: pointer to adapter structure
+ **/
+void ixgbe_ipsec_dump_hw(struct ixgbe_adapter *adapter)
+{
+	struct net_device *dev = adapter->netdev;
+	struct ixgbe_hw *hw = &adapter->hw;
+	int t_idx, i;
+	u32 reg;
+
+	netdev_info(dev, "IPsec HW\n");
+
+	netdev_info(dev, "SECRXCTRL=0x%08x  SECRXSTAT=0x%08x\n",
+		    IXGBE_READ_REG(hw, IXGBE_SECRXCTRL),
+		    IXGBE_READ_REG(hw, IXGBE_SECRXSTAT));
+
+	netdev_info(dev, "SECTXCTRL=0x%08x  SECTXSTAT=0x%08x, SECTXBUFFAF=0x%08x, SECTXMINIFG=0x%08x\n",
+		    IXGBE_READ_REG(hw, IXGBE_SECTXCTRL),
+		    IXGBE_READ_REG(hw, IXGBE_SECTXSTAT),
+		    IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF),
+		    IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG));
+
+	netdev_info(dev, "RxIdx=0x%08x  TxIdx=0x%08x\n",
+		    IXGBE_READ_REG(hw, IXGBE_IPSRXIDX),
+		    IXGBE_READ_REG(hw, IXGBE_IPSTXIDX));
+
+	/* Tx SA Table */
+	netdev_info(dev, "Tx SA HW Table\n");
+	for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_SA_COUNT; t_idx++) {
+		u32 key[4];
+		u32 salt;
+		u32 anybit = 0;
+
+		reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
+		reg &= IXGBE_RXTXIDX_IPS_EN;
+		reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ;
+		IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, reg);
+		for (i = 0; i < 4; i++) {
+			key[i] = IXGBE_READ_REG(hw, IXGBE_IPSTXKEY(i));
+			anybit |= key[i];
+		}
+		salt = IXGBE_READ_REG(hw, IXGBE_IPSTXSALT);
+		anybit |= salt;
+
+		if (anybit)
+			netdev_info(dev, "  SA[%d] key=0x%08x 0x%08x 0x%08x 0x%08x salt=0x%08x\n",
+				    t_idx, key[3], key[2], key[1], key[0],
+				    salt);
+	}
+
+	/* Rx SA Table */
+	netdev_info(dev, "Rx SA HW Table\n");
+	for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_SA_COUNT; t_idx++) {
+		u32 key[4];
+		u32 ip_idx;
+		__be32 spi;
+		u32 salt;
+		u32 mode;
+		u32 anybit = 0;
+
+		reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
+		reg &= IXGBE_RXTXIDX_IPS_EN;
+		reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_SPI;
+		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
+		spi = IXGBE_READ_REG(hw, IXGBE_IPSRXSPI);
+		ip_idx = IXGBE_READ_REG(hw, IXGBE_IPSRXIPIDX);
+
+		reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
+		reg &= IXGBE_RXTXIDX_IPS_EN;
+		reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_KEY;
+		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
+		for (i = 0; i < 4; i++) {
+			key[i] = IXGBE_READ_REG(hw, IXGBE_IPSRXKEY(i));
+			anybit |= key[i];
+		}
+		salt = IXGBE_READ_REG(hw, IXGBE_IPSRXSALT);
+		mode = IXGBE_READ_REG(hw, IXGBE_IPSRXMOD);
+
+		anybit |= salt | mode | spi | ip_idx;
+		if (anybit)
+			netdev_info(dev, "  SA[%d] spi=0x%08x key=0x%08x 0x%08x 0x%08x 0x%08x salt=0x%08x ipi=0x%08x mode=0x%08x\n",
+				    t_idx, spi, key[3], key[2], key[1], key[0],
+				    salt, ip_idx, mode);
+	}
+
+	/* Rx IP Table */
+	netdev_info(dev, "Rx IP HW Table\n");
+	for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; t_idx++) {
+		u32 ipaddr[4];
+		u32 anybit = 0;
+
+		reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
+		reg &= IXGBE_RXTXIDX_IPS_EN;
+		reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_IP;
+		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
+		for (i = 0; i < 4; i++) {
+			ipaddr[i] = IXGBE_READ_REG(hw, IXGBE_IPSRXIPADDR(i));
+			anybit |= ipaddr[i];
+		}
+
+		if (anybit)
+			netdev_info(dev, "  IP[%d] 0x%08x 0x%08x 0x%08x 0x%08x\n",
+				    t_idx, ipaddr[0], ipaddr[1],
+				    ipaddr[2], ipaddr[3]);
+	}
+}
+
+/**
+ * ixgbe_ipsec_set_tx_sa - set the Tx SA registers
+ * @hw: hw specific details
+ * @idx: register index to write
+ * @key: key byte array
+ * @salt: salt bytes
+ **/
+static void ixgbe_ipsec_set_tx_sa(struct ixgbe_hw *hw, u16 idx,
+				  u32 key[], u32 salt)
+{
+	u32 reg;
+	int i;
+
+	for (i = 0; i < 4; i++)
+		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(i), cpu_to_be32(key[3-i]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, cpu_to_be32(salt));
+	IXGBE_WRITE_FLUSH(hw);
+
+	reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
+	reg &= IXGBE_RXTXIDX_IPS_EN;
+	reg |= idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, reg);
+	IXGBE_WRITE_FLUSH(hw);
+}
+
+/**
+ * ixgbe_ipsec_set_rx_item - set an Rx table item
+ * @hw: hw specific details
+ * @idx: register index to write
+ * @tbl: table selector
+ *
+ * Trigger the device to store into a particular Rx table the
+ * data that has already been loaded into the input register
+ **/
+static void ixgbe_ipsec_set_rx_item(struct ixgbe_hw *hw, u16 idx, u32 tbl)
+{
+	u32 reg;
+
+	reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
+	reg &= IXGBE_RXTXIDX_IPS_EN;
+	reg |= tbl | idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
+	IXGBE_WRITE_FLUSH(hw);
+}
+
+/**
+ * ixgbe_ipsec_set_rx_sa - set up the register bits to save SA info
+ * @hw: hw specific details
+ * @idx: register index to write
+ * @spi: security parameter index
+ * @key: key byte array
+ * @salt: salt bytes
+ * @mode: rx decrypt control bits
+ * @ip_idx: index into IP table for related IP address
+ **/
+static void ixgbe_ipsec_set_rx_sa(struct ixgbe_hw *hw, u16 idx, __be32 spi,
+				  u32 key[], u32 salt, u32 mode, u32 ip_idx)
+{
+	int i;
+
+	/* store the SPI (in bigendian) and IPidx */
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, spi);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, ip_idx);
+	IXGBE_WRITE_FLUSH(hw);
+
+	ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_SPI);
+
+	/* store the key, salt, and mode */
+	for (i = 0; i < 4; i++)
+		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(i), cpu_to_be32(key[3-i]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, cpu_to_be32(salt));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, mode);
+	IXGBE_WRITE_FLUSH(hw);
+
+	ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_KEY);
+}
+
+/**
+ * ixgbe_ipsec_set_rx_ip - set up the register bits to save SA IP addr info
+ * @hw: hw specific details
+ * @idx: register index to write
+ * @addr: IP address byte array
+ **/
+static void ixgbe_ipsec_set_rx_ip(struct ixgbe_hw *hw, u16 idx, u32 addr[])
+{
+	int i;
+
+	/* store the ip address */
+	for (i = 0; i < 4; i++)
+		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(i), addr[i]);
+	IXGBE_WRITE_FLUSH(hw);
+
+	ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_IP);
+}
+
+/**
+ * ixgbe_ipsec_clear_hw_tables - because some tables don't get cleared on reset
+ * @hw: hw specific details
+ **/
+void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 buf[4] = {0, 0, 0, 0};
+	u16 idx;
+
+	/* disable Rx and Tx SA lookup */
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
+
+	/* scrub the tables */
+	for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
+		ixgbe_ipsec_set_tx_sa(hw, idx, buf, 0);
+	adapter->num_tx_sa = 0;
+
+	for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
+		ixgbe_ipsec_set_rx_sa(hw, idx, 0, buf, 0, 0, 0);
+	adapter->num_rx_sa = 0;
+
+	for (idx = 0; idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; idx++)
+		ixgbe_ipsec_set_rx_ip(hw, idx, buf);
+}
+
+/**
+ * ixgbe_ipsec_stop_data
+ * @hw: hw specific details
+ **/
+static void ixgbe_ipsec_stop_data(struct ixgbe_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	bool link = adapter->link_up;
+	u32 t_rdy, r_rdy;
+	u32 reg;
+
+	/* halt data paths */
+	reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+	reg |= IXGBE_SECTXCTRL_TX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
+
+	reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+	reg |= IXGBE_SECRXCTRL_RX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
+
+	IXGBE_WRITE_FLUSH(hw);
+
+	/* If the tx fifo doesn't have link, but still has data,
+	 * we can't clear the tx sec block.  Set the MAC loopback
+	 * before block clear
+	 */
+	if (!link) {
+		reg = IXGBE_READ_REG(hw, IXGBE_MACC);
+		reg |= IXGBE_MACC_FLU;
+		IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
+
+		reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+		reg |= IXGBE_HLREG0_LPBK;
+		IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
+
+		IXGBE_WRITE_FLUSH(hw);
+		mdelay(3);
+	}
+
+	/* wait for the paths to empty */
+	do {
+		mdelay(10);
+		t_rdy = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT) &
+			IXGBE_SECTXSTAT_SECTX_RDY;
+		r_rdy = IXGBE_READ_REG(hw, IXGBE_SECRXSTAT) &
+			IXGBE_SECRXSTAT_SECRX_RDY;
+	} while (!t_rdy && !r_rdy);
+
+	/* undo loopback if we played with it earlier */
+	if (!link) {
+		reg = IXGBE_READ_REG(hw, IXGBE_MACC);
+		reg &= ~IXGBE_MACC_FLU;
+		IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
+
+		reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+		reg &= ~IXGBE_HLREG0_LPBK;
+		IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
+
+		IXGBE_WRITE_FLUSH(hw);
+	}
+}
+
+/**
+ * ixgbe_ipsec_stop_engine
+ * @hw: hw specific details
+ **/
+static void ixgbe_ipsec_stop_engine(struct ixgbe_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 reg;
+
+	ixgbe_ipsec_stop_data(adapter);
+
+	/* disable Rx and Tx SA lookup */
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
+
+	/* disable the Rx and Tx engines and full packet store-n-forward */
+	reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+	reg |= IXGBE_SECTXCTRL_SECTX_DIS;
+	reg &= ~IXGBE_SECTXCTRL_STORE_FORWARD;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
+
+	reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+	reg |= IXGBE_SECRXCTRL_SECRX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
+
+	/* restore the "tx security buffer almost full threshold" to 0x250 */
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, 0x250);
+
+	/* Set minimum IFG between packets back to the default 0x1 */
+	reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+	reg = (reg & 0xfffffff0) | 0x1;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
+
+	/* final set for normal (no ipsec offload) processing */
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_SECTX_DIS);
+	IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, IXGBE_SECRXCTRL_SECRX_DIS);
+
+	IXGBE_WRITE_FLUSH(hw);
+}
+
+/**
+ * ixgbe_ipsec_start_engine
+ * @hw: hw specific details
+ *
+ * NOTE: this increases power consumption whether being used or not
+ **/
+static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 reg;
+
+	ixgbe_ipsec_stop_data(adapter);
+
+	/* Set minimum IFG between packets to 3 */
+	reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+	reg = (reg & 0xfffffff0) | 0x3;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
+
+	/* Set "tx security buffer almost full threshold" to 0x15 so that the
+	 * almost full indication is generated only after buffer contains at
+	 * least an entire jumbo packet.
+	 */
+	reg = IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF);
+	reg = (reg & 0xfffffc00) | 0x15;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, reg);
+
+	/* restart the data paths by clearing the DISABLE bits */
+	IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, 0);
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_STORE_FORWARD);
+
+	/* enable Rx and Tx SA lookup */
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, IXGBE_RXTXIDX_IPS_EN);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, IXGBE_RXTXIDX_IPS_EN);
+
+	IXGBE_WRITE_FLUSH(hw);
+}
+
+/**
+ * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset
+ * @hw: hw specific details
+ **/
+void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 zbuf[4] = {0, 0, 0, 0};
+	int i;
+
+	if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED))
+		return;
+
+	/* clean up the engine settings */
+	ixgbe_ipsec_stop_engine(adapter);
+
+	/* start the engine */
+	ixgbe_ipsec_start_engine(adapter);
+
+	/* reload the IP addrs */
+	for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
+		struct rx_ip_sa *ipsa = &adapter->rx_ip_sa_tbl[i];
+
+		if (ipsa->used)
+			ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr);
+		else
+			ixgbe_ipsec_set_rx_ip(hw, i, zbuf);
+	}
+
+	/* reload the Rx keys */
+	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
+		struct rx_sa *rsa = &adapter->rx_sa_tbl[i];
+
+		if (rsa->used)
+			ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi,
+					      rsa->key, rsa->salt,
+					      rsa->mode, rsa->iptbl_ind);
+		else
+			ixgbe_ipsec_set_rx_sa(hw, i, 0, zbuf, 0, 0, 0);
+	}
+
+	/* reload the Tx keys */
+	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
+		struct tx_sa *tsa = &adapter->tx_sa_tbl[i];
+
+		if (tsa->used)
+			ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt);
+		else
+			ixgbe_ipsec_set_tx_sa(hw, i, zbuf, 0);
+	}
+}
+
+/**
+ * ixgbe_ipsec_find_empty_idx - find the first unused security parameter index
+ * @adapter: pointer to adapter struct
+ * @rxtable: true if we need to look in the Rx table
+ *
+ * Returns the first unused index in either the Rx or Tx SA table
+ **/
+static int ixgbe_ipsec_find_empty_idx(struct ixgbe_adapter *adapter,
+				      bool rxtable)
+{
+	u32 i;
+
+	if (rxtable) {
+		if (adapter->num_rx_sa == IXGBE_IPSEC_MAX_SA_COUNT)
+			return -ENOSPC;
+
+		/* search rx sa table */
+		for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
+			if (!adapter->rx_sa_tbl[i].used)
+				return i;
+		}
+	} else {
+		if (adapter->num_rx_sa == IXGBE_IPSEC_MAX_SA_COUNT)
+			return -ENOSPC;
+
+		/* search tx sa table */
+		for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
+			if (!adapter->tx_sa_tbl[i].used)
+				return i;
+		}
+	}
+
+	return -ENOSPC;
+}
+
+/**
+ * ixgbe_ipsec_find_rx_state - find the state that matches
+ * @adapter: pointer to adapter struct
+ * @daddr: inbound address to match
+ * @proto: protocol to match
+ * @spi: SPI to match
+ *
+ * Returns a pointer to the matching SA state information
+ **/
+static struct xfrm_state *ixgbe_ipsec_find_rx_state(
+					struct ixgbe_adapter *adapter,
+					__be32 daddr, u8 proto, __be32 spi)
+{
+	struct rx_sa *rsa;
+	u32 i;
+
+	/* search rx sa table */
+	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
+		rsa = &adapter->rx_sa_tbl[i];
+
+		if (!rsa->used)
+			continue;
+
+		if (spi == rsa->xs->id.spi &&
+		    daddr == rsa->xs->id.daddr.a4 &&
+		    proto == rsa->xs->id.proto) {
+			xfrm_state_hold(adapter->rx_sa_tbl[i].xs);
+			return adapter->rx_sa_tbl[i].xs;
+		}
+	}
+
+	return NULL;
+}
+
+/**
+ * ixgbe_ipsec_parse_proto_keys - find the key and salt based on the protocol
+ * @xs: pointer to xfrm_state struct
+ * @mykey: pointer to key array to populate
+ * @mysalt: pointer to salt value to populate
+ *
+ * This copies the protocol keys and salt to our own data tables.  The
+ * 82599 family only supports the one algorithm.
+ **/
+static int ixgbe_ipsec_parse_proto_keys(struct xfrm_state *xs,
+					u32 *mykey, u32 *mysalt)
+{
+	struct net_device *dev = xs->xso.dev;
+	unsigned char *key_data;
+	char *alg_name = NULL;
+	char *aes_gcm_name = "rfc4106(gcm(aes))";
+	int key_len;
+
+	if (xs->aead) {
+		key_data = &xs->aead->alg_key[0];
+		key_len = xs->aead->alg_key_len;
+		alg_name = xs->aead->alg_name;
+	} else {
+		netdev_err(dev, "Unsupported IPsec algorithm\n");
+		return -EINVAL;
+	}
+
+	if (strcmp(alg_name, aes_gcm_name)) {
+		netdev_err(dev, "Unsupported IPsec algorithm - please use %s\n",
+			   aes_gcm_name);
+		return -EINVAL;
+	}
+
+	/* 160 accounts for 16 byte key and 4 byte salt */
+	if (key_len == 128) {
+		netdev_info(dev, "IPsec hw offload parameters missing 32 bit salt value\n");
+	} else if (key_len != 160) {
+		netdev_err(dev, "IPsec hw offload only supports keys up to 128 bits with a 32 bit salt\n");
+		return -EINVAL;
+	}
+
+	/* The key bytes come down in a bigendian array of bytes, and
+	 * salt is always the last 4 bytes of the key array.
+	 * We don't need to do any byteswapping.
+	 */
+	memcpy(mykey, key_data, 16);
+	if (key_len == 160)
+		*mysalt = ((u32 *)key_data)[4];
+	else
+		*mysalt = 0;
+
+	return 0;
+}
+
+/**
+ * ixgbe_ipsec_add_sa - program device with a security association
+ * @xs: pointer to transformer state struct
+ **/
+static int ixgbe_ipsec_add_sa(struct xfrm_state *xs)
+{
+	struct net_device *dev = xs->xso.dev;
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	int checked, match, first;
+	u32 sa_idx;
+	int ret;
+	int i;
+
+	if (xs->id.proto != IPPROTO_ESP && xs->id.proto != IPPROTO_AH) {
+		netdev_err(dev, "Unsupported protocol 0x%04x for ipsec offload\n",
+			   xs->id.proto);
+		return -EINVAL;
+	}
+
+	if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
+		struct rx_sa rsa;
+
+		if (xs->calg) {
+			netdev_err(dev, "Compression offload not supported\n");
+			return -EINVAL;
+		}
+
+		/* find the first unused index */
+		sa_idx = ixgbe_ipsec_find_empty_idx(adapter, true);
+		if (sa_idx < 0) {
+			netdev_err(dev, "No space for SA in Rx table!\n");
+			return -ENOSPC;
+		}
+
+		memset(&rsa, 0, sizeof(rsa));
+		rsa.used = true;
+		rsa.xs = xs;
+
+		if (rsa.xs->id.proto & IPPROTO_ESP)
+			rsa.decrypt = xs->ealg || xs->aead;
+
+		/* get the key and salt */
+		ret = ixgbe_ipsec_parse_proto_keys(xs, rsa.key, &rsa.salt);
+		if (ret) {
+			netdev_err(dev, "Failed to get key data for Rx SA table\n");
+			return ret;
+		}
+
+		/* get ip for rx sa table */
+		if (xs->xso.flags & XFRM_OFFLOAD_IPV6)
+			memcpy(rsa.ipaddr, &xs->id.daddr.a6, 16);
+		else
+			memcpy(&rsa.ipaddr[3], &xs->id.daddr.a4, 4);
+
+		/* The HW does not have a 1:1 mapping from keys to IP addrs, so
+		 * check for a matching IP addr entry in the table.  If the addr
+		 * already exists, use it; else find an unused slot and add the
+		 * addr.  If one does not exist and there are no unused table
+		 * entries, fail the request.
+		 */
+
+		/* Find an existing match or first not used, and stop looking
+		 * after we've checked all we know we have.
+		 */
+		checked = 0;
+		match = -1;
+		first = -1;
+		for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT &&
+			    (checked < adapter->num_rx_sa || first < 0); i++) {
+			if (adapter->rx_ip_sa_tbl[i].used) {
+				if (!memcmp(adapter->rx_ip_sa_tbl[i].ipaddr,
+					    rsa.ipaddr, sizeof(rsa.ipaddr))) {
+					match = i;
+					break;
+				}
+				checked++;
+			} else if (first < 0) {
+				first = i;  /* track the first empty seen */
+			}
+		}
+
+		if (adapter->num_rx_sa == 0)
+			first = 0;
+
+		if (match >= 0) {
+			/* addrs are the same, we should use this one */
+			rsa.iptbl_ind = match;
+			adapter->rx_ip_sa_tbl[match].ref_cnt++;
+
+		} else if (first >= 0) {
+			/* no matches, but here's an empty slot */
+			rsa.iptbl_ind = first;
+
+			memcpy(adapter->rx_ip_sa_tbl[first].ipaddr,
+			       rsa.ipaddr, sizeof(rsa.ipaddr));
+			adapter->rx_ip_sa_tbl[first].ref_cnt = 1;
+			adapter->rx_ip_sa_tbl[first].used = true;
+
+			ixgbe_ipsec_set_rx_ip(hw, rsa.iptbl_ind, rsa.ipaddr);
+
+		} else {
+			/* no match and no empty slot */
+			netdev_err(dev, "No space for SA in Rx IP SA table\n");
+			memset(&rsa, 0, sizeof(rsa));
+			return -ENOSPC;
+		}
+
+		rsa.mode = IXGBE_RXMOD_VALID;
+		if (rsa.xs->id.proto & IPPROTO_ESP)
+			rsa.mode |= IXGBE_RXMOD_PROTO_ESP;
+		if (rsa.decrypt)
+			rsa.mode |= IXGBE_RXMOD_DECRYPT;
+		if (rsa.xs->xso.flags & XFRM_OFFLOAD_IPV6)
+			rsa.mode |= IXGBE_RXMOD_IPV6;
+
+		/* the preparations worked, so save the info */
+		memcpy(&adapter->rx_sa_tbl[sa_idx], &rsa, sizeof(rsa));
+
+		ixgbe_ipsec_set_rx_sa(hw, sa_idx, rsa.xs->id.spi, rsa.key,
+				      rsa.salt, rsa.mode, rsa.iptbl_ind);
+		xs->xso.offload_handle = sa_idx + IXGBE_IPSEC_BASE_RX_INDEX;
+
+		adapter->num_rx_sa++;
+	} else {
+		struct tx_sa tsa;
+
+		/* find the first unused index */
+		sa_idx = ixgbe_ipsec_find_empty_idx(adapter, false);
+		if (sa_idx < 0) {
+			netdev_err(dev, "No space for SA in Tx table\n");
+			return -ENOSPC;
+		}
+
+		memset(&tsa, 0, sizeof(tsa));
+		tsa.used = true;
+		tsa.xs = xs;
+
+		if (xs->id.proto & IPPROTO_ESP)
+			tsa.encrypt = xs->ealg || xs->aead;
+
+		ret = ixgbe_ipsec_parse_proto_keys(xs, tsa.key, &tsa.salt);
+		if (ret) {
+			netdev_err(dev, "Failed to get key data for Tx SA table\n");
+			memset(&tsa, 0, sizeof(tsa));
+			return ret;
+		}
+
+		/* the preparations worked, so save the info */
+		memcpy(&adapter->tx_sa_tbl[sa_idx], &tsa, sizeof(tsa));
+
+		ixgbe_ipsec_set_tx_sa(hw, sa_idx, tsa.key, tsa.salt);
+
+		xs->xso.offload_handle = sa_idx + IXGBE_IPSEC_BASE_TX_INDEX;
+
+		adapter->num_tx_sa++;
+	}
+
+	/* enable the engine if not already warmed up */
+	if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)) {
+		ixgbe_ipsec_start_engine(adapter);
+		adapter->flags2 |= IXGBE_FLAG2_IPSEC_ENABLED;
+	}
+
+	return 0;
+}
+
+/**
+ * ixgbe_ipsec_del_sa - clear out this specific SA
+ * @xs: pointer to transformer state struct
+ **/
+static void ixgbe_ipsec_del_sa(struct xfrm_state *xs)
+{
+	struct net_device *dev = xs->xso.dev;
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 zerobuf[4] = {0, 0, 0, 0};
+	u16 sa_idx;
+
+	if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
+		struct rx_sa *rsa;
+		u8 iptbl_ind;
+
+		sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_RX_INDEX;
+		rsa = &adapter->rx_sa_tbl[sa_idx];
+
+		if (!rsa->used) {
+			netdev_err(dev, "Invalid Rx SA selected sa_idx=%d offload_handle=%lu\n",
+				   sa_idx, xs->xso.offload_handle);
+			return;
+		}
+
+		/* if the IP table entry is referenced by only this SA,
+		 * i.e. ref_cnt is only 1, clear the IP table entry as well
+		 */
+		iptbl_ind = rsa->iptbl_ind;
+		if (adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt > 0) {
+			adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt--;
+
+			if (!adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt) {
+				memset(&adapter->rx_ip_sa_tbl[iptbl_ind], 0,
+				       sizeof(struct rx_ip_sa));
+				ixgbe_ipsec_set_rx_ip(hw, iptbl_ind, zerobuf);
+			}
+		}
+
+		/* clear the SA table entry */
+		memset(rsa, 0, sizeof(struct rx_sa));
+		ixgbe_ipsec_set_rx_sa(hw, sa_idx, 0, zerobuf, 0, 0, 0);
+
+		adapter->num_rx_sa--;
+
+	} else {
+		sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
+
+		if (!adapter->tx_sa_tbl[sa_idx].used) {
+			netdev_err(dev, "Invalid Tx SA selected sa_idx=%d offload_handle=%lu\n",
+				   sa_idx, xs->xso.offload_handle);
+			return;
+		}
+
+		/* clear the SA table entry */
+		memset(&adapter->tx_sa_tbl[sa_idx], 0, sizeof(struct tx_sa));
+		ixgbe_ipsec_set_tx_sa(hw, sa_idx, zerobuf, 0);
+
+		adapter->num_tx_sa--;
+	}
+
+	/* if there are no SAs left, stop the engine to save energy */
+	if (adapter->num_rx_sa == 0 && adapter->num_tx_sa == 0) {
+		adapter->flags2 &= ~IXGBE_FLAG2_IPSEC_ENABLED;
+		ixgbe_ipsec_stop_engine(adapter);
+	}
+}
+
+/**
+ * ixgbe_ipsec_free - called by xfrm garbage collections
+ * @xs: pointer to transformer state struct
+ *
+ * We don't have any garbage to collect, so we shouldn't even bother
+ * implementing this function.
+ **/
+static void ixgbe_ipsec_free(struct xfrm_state *xs)
+{
+}
+
+/**
+ * ixgbe_ipsec_offload_ok - can this packet use the xfrm hw offload
+ * @skb: current data packet
+ * @xs: pointer to transformer state struct
+ **/
+static bool ixgbe_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
+{
+	if (xs->props.family == AF_INET) {
+		/* Offload with IPv4 options is not supported yet */
+		if (ip_hdr(skb)->ihl > 5)
+			return false;
+	} else {
+		/* Offload with IPv6 extension headers is not support yet */
+		if (ipv6_ext_hdr(ipv6_hdr(skb)->nexthdr))
+			return false;
+	}
+
+	return true;
+}
+
+static const struct xfrmdev_ops ixgbe_xfrmdev_ops = {
+	.xdo_dev_state_add = ixgbe_ipsec_add_sa,
+	.xdo_dev_state_delete = ixgbe_ipsec_del_sa,
+	.xdo_dev_state_free = ixgbe_ipsec_free,
+	.xdo_dev_offload_ok = ixgbe_ipsec_offload_ok,
+};
+
+/**
+ * ixgbe_ipsec_tx - setup Tx flags for ipsec offload
+ * @tx_ring: outgoing context
+ * @skb: current data packet
+ * @itd: ipsec Tx data for later use in building context descriptor
+ **/
+int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
+		   struct ixgbe_ipsec_tx_data *itd)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(tx_ring->netdev);
+	struct xfrm_state *xs;
+	struct tx_sa *tsa;
+
+	/* TODO: not supporting IPv6 yet */
+	if (skb->protocol != htons(ETH_P_IP))
+		return 0;
+
+	if ((ip_hdr(skb)->protocol != IPPROTO_ESP) &&
+	    (ip_hdr(skb)->protocol != IPPROTO_AH))
+		return 0;
+
+	if (!skb->sp) {
+		netdev_err(tx_ring->netdev, "%s: no xfrm state skb->sp = %p encap=%d skb=%p\n",
+			   __func__, skb->sp, skb->encapsulation, skb);
+		return 0;
+	}
+
+	if (!skb->sp->len) {
+		netdev_err(tx_ring->netdev, "%s: no xfrm state len = %d\n",
+			   __func__, skb->sp->len);
+		return 0;
+	}
+
+	xs = xfrm_input_state(skb);
+	if (!xs) {
+		netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs = %p\n",
+			   __func__, xs);
+		return 0;
+	}
+
+	itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
+	if (itd->sa_idx < 0 || itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
+		netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d\n",
+			   __func__, itd->sa_idx);
+		return 0;
+	}
+
+	tsa = &adapter->tx_sa_tbl[itd->sa_idx];
+	if (!tsa->used) {
+		netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
+			   __func__, itd->sa_idx);
+		return 0;
+	}
+
+	itd->flags = 0;
+	if (xs->id.proto == IPPROTO_ESP) {
+		itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP |
+			      IXGBE_ADVTXD_TUCMD_L4T_TCP;
+		if (skb->protocol == htons(ETH_P_IP))
+			itd->flags |= IXGBE_ADVTXD_TUCMD_IPV4;
+		itd->trailer_len = xs->props.trailer_len;
+	}
+	if (tsa->encrypt)
+		itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN;
+
+	return 1;
+}
+
+/**
+ * ixgbe_ipsec_process_rx_offload - decode ipsec bits from Rx descriptor
+ * @rx_ring: receiving ring
+ * @rx_desc: receive data descriptor
+ * @skb: current data packet
+ *
+ * Determine if there was an ipsec encapsulation noticed, and if so set up
+ * the resulting status for later in the receive stack.
+ **/
+void ixgbe_ipsec_process_rx_offload(struct ixgbe_ring *rx_ring,
+				    union ixgbe_adv_rx_desc *rx_desc,
+				    struct sk_buff *skb)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(rx_ring->netdev);
+	u16 pkt_info = le16_to_cpu(rx_desc->wb.lower.lo_dword.hs_rss.pkt_info);
+	u32 status_error = le32_to_cpu(rx_desc->wb.upper.status_error);
+	u16 ipsec_pkt_types = IXGBE_RXDADV_PKTTYPE_IPSEC_AH |
+				IXGBE_RXDADV_PKTTYPE_IPSEC_ESP;
+	struct xfrm_offload *xo = NULL;
+	struct xfrm_state *xs = NULL;
+	struct iphdr *iph;
+	u32 crypto_status = 0;
+	u8 *c_hdr;
+	__be32 spi;
+	u8 proto;
+
+	/* TODO: we're only handling ipv4 at the moment */
+	if (!(pkt_info & IXGBE_RXDADV_PKTTYPE_IPV4))
+		return;
+
+	/* look for our packet types */
+	if (!(pkt_info & ipsec_pkt_types))
+		return;
+
+	/* don't process this if no offload work was done */
+	if (!(status_error & IXGBE_RXDADV_STAT_SECP))
+		return;
+
+	iph = (struct iphdr *)(skb_mac_header(skb) + ETH_HLEN);
+	c_hdr = (u8 *)iph + iph->ihl * 4;
+	switch (pkt_info & ipsec_pkt_types) {
+	case IXGBE_RXDADV_PKTTYPE_IPSEC_AH:
+		spi = ((struct ip_auth_hdr *)c_hdr)->spi;
+		proto = IPPROTO_AH;
+		break;
+	case IXGBE_RXDADV_PKTTYPE_IPSEC_ESP:
+		spi = ((struct ip_esp_hdr *)c_hdr)->spi;
+		proto = IPPROTO_ESP;
+		break;
+	default:
+		spi = 0;
+		proto = 0;
+		break;
+	}
+
+	xs = ixgbe_ipsec_find_rx_state(adapter, iph->daddr, proto, spi);
+	if (unlikely(!xs))
+		return;
+
+	/* decode error bits, if any */
+	if ((status_error & IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) == IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) {
+		if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_AH)
+			crypto_status = CRYPTO_TRANSPORT_AH_AUTH_FAILED;
+		else if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_ESP)
+			crypto_status = CRYPTO_TRANSPORT_ESP_AUTH_FAILED;
+	} else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL) {
+		crypto_status = CRYPTO_INVALID_PROTOCOL;
+	} else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH) {
+		crypto_status = CRYPTO_INVALID_PACKET_SYNTAX;
+	} else {
+		crypto_status = CRYPTO_SUCCESS;
+	}
+
+	skb->sp = secpath_dup(skb->sp);
+	if (unlikely(!skb->sp))
+		return;
+
+	skb->sp->xvec[skb->sp->len++] = xs;
+	skb->sp->olen++;
+	xo = xfrm_offload(skb);
+	xo->flags = CRYPTO_DONE;
+	xo->status = crypto_status;
+
+	if (xo->status == CRYPTO_SUCCESS)
+		rx_ring->rx_stats.ipsec_offloads++;
+	else
+		rx_ring->rx_stats.ipsec_offload_faileds++;
+}
+
+/**
+ * ixgbe_init_ipsec_offload - initialize security registers for IPSec operation
+ * @adapter: board private structure
+ **/
+void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter)
+{
+	// TODO: enable and support checksum offload with ESP
+	//netdev_features_t esp_feats = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM;
+	netdev_features_t esp_feats = NETIF_F_HW_ESP;
+	size_t size;
+
+	/* allocate SA tables for rx, tx, and ip table */
+	size = sizeof(struct rx_sa) * IXGBE_IPSEC_MAX_SA_COUNT;
+	adapter->rx_sa_tbl = kzalloc(size, GFP_KERNEL);
+	if (!adapter->rx_sa_tbl)
+		goto err;
+
+	size = sizeof(struct tx_sa) * IXGBE_IPSEC_MAX_SA_COUNT;
+	adapter->tx_sa_tbl = kzalloc(size, GFP_KERNEL);
+	if (!adapter->tx_sa_tbl)
+		goto err;
+
+	size = sizeof(struct rx_ip_sa) * IXGBE_IPSEC_MAX_RX_IP_COUNT;
+	adapter->rx_ip_sa_tbl = kzalloc(size, GFP_KERNEL);
+	if (!adapter->rx_ip_sa_tbl)
+		goto err;
+
+	adapter->num_rx_sa = 0;
+	adapter->num_tx_sa = 0;
+	ixgbe_ipsec_clear_hw_tables(adapter);
+	ixgbe_ipsec_stop_engine(adapter);
+
+	adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops;
+	adapter->netdev->features |= esp_feats;
+	adapter->netdev->hw_enc_features |= esp_feats;
+
+	return;
+
+err:
+	kfree(adapter->rx_ip_sa_tbl);
+	kfree(adapter->rx_sa_tbl);
+	kfree(adapter->tx_sa_tbl);
+	netdev_err(adapter->netdev, "Unable to allocate memory for SA tables");
+}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
new file mode 100644
index 0000000..265950d
--- /dev/null
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
@@ -0,0 +1,80 @@
+/*******************************************************************************
+
+  Intel 10 Gigabit PCI Express Linux driver
+  Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
+
+  This program is free software; you can redistribute it and/or modify it
+  under the terms and conditions of the GNU General Public License,
+  version 2, as published by the Free Software Foundation.
+
+  This program is distributed in the hope it will be useful, but WITHOUT
+  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+  more details.
+
+  You should have received a copy of the GNU General Public License along with
+  this program.  If not, see <http://www.gnu.org/licenses/>.
+
+  The full GNU General Public License is included in this distribution in
+  the file called "COPYING".
+
+  Contact Information:
+  Linux NICS <linux.nics@intel.com>
+  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
+  Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+
+*******************************************************************************/
+
+#ifndef _IXGBE_IPSEC_H_
+#define _IXGBE_IPSEC_H_
+
+#define IXGBE_IPSEC_MAX_SA_COUNT	1024
+#define IXGBE_IPSEC_MAX_RX_IP_COUNT	128
+#define IXGBE_IPSEC_BASE_RX_INDEX	IXGBE_IPSEC_MAX_SA_COUNT
+#define IXGBE_IPSEC_BASE_TX_INDEX	(IXGBE_IPSEC_MAX_SA_COUNT * 2)
+
+#define IXGBE_RXTXIDX_IPS_EN		0x00000001
+#define IXGBE_RXIDX_TBL_MASK		0x00000006
+#define IXGBE_RXIDX_TBL_IP		0x00000002
+#define IXGBE_RXIDX_TBL_SPI		0x00000004
+#define IXGBE_RXIDX_TBL_KEY		0x00000006
+#define IXGBE_RXTXIDX_IDX_MASK		0x00001ff8
+#define IXGBE_RXTXIDX_IDX_READ		0x40000000
+#define IXGBE_RXTXIDX_IDX_WRITE		0x80000000
+
+#define IXGBE_RXMOD_VALID		0x00000001
+#define IXGBE_RXMOD_PROTO_ESP		0x00000004
+#define IXGBE_RXMOD_DECRYPT		0x00000008
+#define IXGBE_RXMOD_IPV6		0x00000010
+
+struct rx_sa {
+	struct xfrm_state *xs;
+	u32 ipaddr[4];
+	u32 key[4];
+	u32 salt;
+	u32 mode;
+	u8  iptbl_ind;
+	bool used;
+	bool decrypt;
+};
+
+struct rx_ip_sa {
+	u32 ipaddr[4];
+	u32 ref_cnt;
+	bool used;
+};
+
+struct tx_sa {
+	struct xfrm_state *xs;
+	u32 key[4];
+	u32 salt;
+	bool encrypt;
+	bool used;
+};
+
+struct ixgbe_ipsec_tx_data {
+	u32 flags;
+	u16 trailer_len;
+	u16 sa_idx;
+};
+#endif /* _IXGBE_IPSEC_H_ */
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 4/9] ixgbe: add ipsec data to driver struct

[Shannon Nelson]

Add the ipsec table definitions to the driver's adapter struct and
add the new code into the Makefile.  This sets the stage for
actually using the new code.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/Makefile |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe.h  | 43 +++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile
index 35e6fa6..8319465 100644
--- a/drivers/net/ethernet/intel/ixgbe/Makefile
+++ b/drivers/net/ethernet/intel/ixgbe/Makefile
@@ -42,3 +42,4 @@ ixgbe-$(CONFIG_IXGBE_DCB) +=  ixgbe_dcb.o ixgbe_dcb_82598.o \
 ixgbe-$(CONFIG_IXGBE_HWMON) += ixgbe_sysfs.o
 ixgbe-$(CONFIG_DEBUG_FS) += ixgbe_debugfs.o
 ixgbe-$(CONFIG_FCOE:m=y) += ixgbe_fcoe.o
+ixgbe-$(CONFIG_XFRM_OFFLOAD) += ixgbe_ipsec.o
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 468c355..f324271 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -52,6 +52,9 @@
 #ifdef CONFIG_IXGBE_DCA
 #include <linux/dca.h>
 #endif
+#ifdef CONFIG_XFRM_OFFLOAD
+#include "ixgbe_ipsec.h"
+#endif /* CONFIG_XFRM_OFFLOAD */
 
 #include <net/busy_poll.h>
 
@@ -170,10 +173,11 @@ enum ixgbe_tx_flags {
 	IXGBE_TX_FLAGS_CC	= 0x08,
 	IXGBE_TX_FLAGS_IPV4	= 0x10,
 	IXGBE_TX_FLAGS_CSUM	= 0x20,
+	IXGBE_TX_FLAGS_IPSEC    = 0x40,
 
 	/* software defined flags */
-	IXGBE_TX_FLAGS_SW_VLAN	= 0x40,
-	IXGBE_TX_FLAGS_FCOE	= 0x80,
+	IXGBE_TX_FLAGS_SW_VLAN	= 0x80,
+	IXGBE_TX_FLAGS_FCOE	= 0x100,
 };
 
 /* VLAN info */
@@ -279,6 +283,10 @@ struct ixgbe_rx_queue_stats {
 	u64 alloc_rx_page_failed;
 	u64 alloc_rx_buff_failed;
 	u64 csum_err;
+#if CONFIG_XFRM_OFFLOAD
+	u64 ipsec_offloads;
+	u64 ipsec_offload_faileds;
+#endif /* CONFIG_XFRM_OFFLOAD */
 };
 
 #define IXGBE_TS_HDR_LEN 8
@@ -629,6 +637,8 @@ struct ixgbe_adapter {
 #define IXGBE_FLAG2_EEE_CAPABLE			BIT(14)
 #define IXGBE_FLAG2_EEE_ENABLED			BIT(15)
 #define IXGBE_FLAG2_RX_LEGACY			BIT(16)
+#define IXGBE_FLAG2_IPSEC_CAPABLE		BIT(17)
+#define IXGBE_FLAG2_IPSEC_ENABLED		BIT(18)
 
 	/* Tx fast path data */
 	int num_tx_queues;
@@ -780,6 +790,14 @@ struct ixgbe_adapter {
 
 #define IXGBE_RSS_KEY_SIZE     40  /* size of RSS Hash Key in bytes */
 	u32 *rss_key;
+
+#ifdef CONFIG_XFRM
+	u16 num_rx_sa;
+	u16 num_tx_sa;
+	struct rx_ip_sa *rx_ip_sa_tbl;
+	struct rx_sa *rx_sa_tbl;
+	struct tx_sa *tx_sa_tbl;
+#endif /* CONFIG_XFRM */
 };
 
 static inline u8 ixgbe_max_rss_indices(struct ixgbe_adapter *adapter)
@@ -1010,4 +1028,25 @@ void ixgbe_store_key(struct ixgbe_adapter *adapter);
 void ixgbe_store_reta(struct ixgbe_adapter *adapter);
 s32 ixgbe_negotiate_fc(struct ixgbe_hw *hw, u32 adv_reg, u32 lp_reg,
 		       u32 adv_sym, u32 adv_asm, u32 lp_sym, u32 lp_asm);
+
+#ifdef CONFIG_XFRM_OFFLOAD
+void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter);
+void ixgbe_ipsec_process_rx_offload(struct ixgbe_ring *rx_ring,
+				    union ixgbe_adv_rx_desc *rx_desc,
+				    struct sk_buff *skb);
+int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
+		   struct ixgbe_ipsec_tx_data *itd);
+void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter);
+void ixgbe_ipsec_dump_info(struct ixgbe_adapter *adapter);
+void ixgbe_ipsec_dump_hw(struct ixgbe_adapter *adapter);
+void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter);
+#else
+inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) { };
+inline void ixgbe_ipsec_process_rx_offload(struct ixgbe_ring *rx_ring,
+				    union ixgbe_adv_rx_desc *rx_desc,
+				    struct sk_buff *skb) { };
+inline int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
+		   struct ixgbe_ipsec_tx_data *itd) { return 0; };
+inline void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) { };
+#endif /* CONFIG_XFRM_OFFLOAD */
 #endif /* _IXGBE_H_ */
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 5/9] ixgbe: add ipsec offload stats

[Shannon Nelson]

Add the ipsec offload and offload_failed queue data to the ethtool stats
output.  I'm not thrilled with adding these to every single queue, but
for the moment they're useful for debugging.  This may not be a part of
the final patchset.

While we're here, fix up a little array-index management to make it a
little less likely to forget to update.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 31 +++++++++++++-----------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 0aad1c2..e798f02 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -134,9 +134,10 @@ static const struct ixgbe_stats ixgbe_gstrings_stats[] = {
  */
 #define IXGBE_NUM_RX_QUEUES netdev->num_tx_queues
 
-#define IXGBE_QUEUE_STATS_LEN ( \
+#define IXGBE_QUEUE_STATS_LEN (( \
 	(netdev->num_tx_queues + IXGBE_NUM_RX_QUEUES) * \
-	(sizeof(struct ixgbe_queue_stats) / sizeof(u64)))
+	(sizeof(struct ixgbe_queue_stats) / sizeof(u64))) + \
+	(2 * IXGBE_NUM_RX_QUEUES))
 #define IXGBE_GLOBAL_STATS_LEN ARRAY_SIZE(ixgbe_gstrings_stats)
 #define IXGBE_PB_STATS_LEN ( \
 			(sizeof(((struct ixgbe_adapter *)0)->stats.pxonrxc) + \
@@ -1234,34 +1235,32 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 	for (j = 0; j < netdev->num_tx_queues; j++) {
 		ring = adapter->tx_ring[j];
 		if (!ring) {
-			data[i] = 0;
-			data[i+1] = 0;
-			i += 2;
+			data[i++] = 0;
+			data[i++] = 0;
 			continue;
 		}
 
 		do {
 			start = u64_stats_fetch_begin_irq(&ring->syncp);
-			data[i]   = ring->stats.packets;
-			data[i+1] = ring->stats.bytes;
+			data[i++]   = ring->stats.packets;
+			data[i++] = ring->stats.bytes;
 		} while (u64_stats_fetch_retry_irq(&ring->syncp, start));
-		i += 2;
 	}
 	for (j = 0; j < IXGBE_NUM_RX_QUEUES; j++) {
 		ring = adapter->rx_ring[j];
 		if (!ring) {
-			data[i] = 0;
-			data[i+1] = 0;
-			i += 2;
+			data[i++] = 0;
+			data[i++] = 0;
 			continue;
 		}
 
 		do {
 			start = u64_stats_fetch_begin_irq(&ring->syncp);
-			data[i]   = ring->stats.packets;
-			data[i+1] = ring->stats.bytes;
+			data[i++] = ring->stats.packets;
+			data[i++] = ring->stats.bytes;
+			data[i++] = ring->rx_stats.ipsec_offloads;
+			data[i++] = ring->rx_stats.ipsec_offload_faileds;
 		} while (u64_stats_fetch_retry_irq(&ring->syncp, start));
-		i += 2;
 	}
 
 	for (j = 0; j < IXGBE_MAX_PACKET_BUFFERS; j++) {
@@ -1304,6 +1303,10 @@ static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
 			p += ETH_GSTRING_LEN;
 			sprintf(p, "rx_queue_%u_bytes", i);
 			p += ETH_GSTRING_LEN;
+			sprintf(p, "rx_queue_%u_ipsec_offloads", i);
+			p += ETH_GSTRING_LEN;
+			sprintf(p, "rx_queue_%u_ipsec_offload_faileds", i);
+			p += ETH_GSTRING_LEN;
 		}
 		for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
 			sprintf(p, "tx_pb_%u_pxon", i);
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 6/9] ixgbe: add ipsec debug helpers

[Shannon Nelson]

These are a couple of debugfs commands to inspect and manage the ipsec
tables.  I really don't expect this patch to be in the final patchset.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 5e2c1e3..0d0a9f0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -120,11 +120,27 @@ static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
 		} else {
 			e_dev_info("read <reg>\n");
 		}
+#ifdef CONFIG_XFRM_OFFLOAD
+	} else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-sw-dump", 13) == 0) {
+		ixgbe_ipsec_dump_info(adapter);
+	} else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-hw-dump", 13) == 0) {
+		ixgbe_ipsec_dump_hw(adapter);
+	} else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-hw-clean", 14) == 0) {
+		ixgbe_ipsec_clear_hw_tables(adapter);
+	} else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-restore", 13) == 0) {
+		ixgbe_ipsec_restore(adapter);
+#endif /* CONFIG_XFRM_OFFLOAD */
 	} else {
 		e_dev_info("Unknown command %s\n", ixgbe_dbg_reg_ops_buf);
 		e_dev_info("Available commands:\n");
 		e_dev_info("   read <reg>\n");
 		e_dev_info("   write <reg> <value>\n");
+#ifdef CONFIG_XFRM_OFFLOAD
+		e_dev_info("   ipsec-sw-dump\n");
+		e_dev_info("   ipsec-hw-dump\n");
+		e_dev_info("   ipsec-hw-clean\n");
+		e_dev_info("   ipsec-restore\n");
+#endif /* CONFIG_XFRM_OFFLOAD */
 	}
 	return count;
 }
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 7/9] ixgbe: add tx ipsec offload

[Shannon Nelson]

Add the Tx offload mechanisms.

Currently this is slightly broken and not setting up the Tx checksum
offload correctly, so is best used with
	ethtool -K ethN tx off

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 44 ++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 7f503d3..a434675 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2423,8 +2423,8 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				continue;
 			}
 		}
-
 #endif /* IXGBE_FCOE */
+
 		ixgbe_rx_skb(q_vector, skb);
 
 		/* update budget accounting */
@@ -7791,9 +7791,10 @@ static void ixgbe_service_task(struct work_struct *work)
 
 static int ixgbe_tso(struct ixgbe_ring *tx_ring,
 		     struct ixgbe_tx_buffer *first,
-		     u8 *hdr_len)
+		     u8 *hdr_len,
+		     struct ixgbe_ipsec_tx_data *itd)
 {
-	u32 vlan_macip_lens, type_tucmd, mss_l4len_idx;
+	u32 vlan_macip_lens, type_tucmd, mss_l4len_idx, fcoe_sof_eof = 0;
 	struct sk_buff *skb = first->skb;
 	union {
 		struct iphdr *v4;
@@ -7872,7 +7873,14 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
 	vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT;
 	vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
 
-	ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd,
+#ifdef CONFIG_XFRM_OFFLOAD
+	if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
+		fcoe_sof_eof |= itd->sa_idx;
+		type_tucmd |= itd->flags | itd->trailer_len;
+	}
+#endif /* CONFIG_XFRM_OFFLOAD */
+
+	ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fcoe_sof_eof, type_tucmd,
 			  mss_l4len_idx);
 
 	return 1;
@@ -7888,10 +7896,12 @@ static inline bool ixgbe_ipv6_csum_is_sctp(struct sk_buff *skb)
 }
 
 static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
-			  struct ixgbe_tx_buffer *first)
+			  struct ixgbe_tx_buffer *first,
+			  struct ixgbe_ipsec_tx_data *itd)
 {
 	struct sk_buff *skb = first->skb;
 	u32 vlan_macip_lens = 0;
+	u32 fcoe_sof_eof = 0;
 	u32 type_tucmd = 0;
 
 	if (skb->ip_summed != CHECKSUM_PARTIAL) {
@@ -7932,7 +7942,15 @@ static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
 	vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT;
 	vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
 
-	ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, 0);
+#ifdef CONFIG_XFRM_OFFLOAD
+	if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
+		fcoe_sof_eof |= itd->sa_idx;
+		type_tucmd |= itd->flags | itd->trailer_len;
+	}
+#endif /* CONFIG_XFRM_OFFLOAD */
+
+	ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens,
+			  fcoe_sof_eof, type_tucmd, 0);
 }
 
 #define IXGBE_SET_FLAG(_input, _flag, _result) \
@@ -7980,6 +7998,11 @@ static void ixgbe_tx_olinfo_status(union ixgbe_adv_tx_desc *tx_desc,
 					IXGBE_TX_FLAGS_IPV4,
 					IXGBE_ADVTXD_POPTS_IXSM);
 
+	/* enble IPsec */
+	olinfo_status |= IXGBE_SET_FLAG(tx_flags,
+					IXGBE_TX_FLAGS_IPSEC,
+					IXGBE_ADVTXD_POPTS_IPSEC);
+
 	/*
 	 * Check Context must be set if Tx switch is enabled, which it
 	 * always is for case where virtual functions are running
@@ -8444,6 +8467,7 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
 	u32 tx_flags = 0;
 	unsigned short f;
 	u16 count = TXD_USE_COUNT(skb_headlen(skb));
+	struct ixgbe_ipsec_tx_data ipsec_tx;
 	__be16 protocol = skb->protocol;
 	u8 hdr_len = 0;
 
@@ -8532,6 +8556,10 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
 		}
 	}
 
+	if ((adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED) &&
+	    ixgbe_ipsec_tx(tx_ring, skb, &ipsec_tx))
+		tx_flags |= IXGBE_TX_FLAGS_IPSEC | IXGBE_TX_FLAGS_CC;
+
 	/* record initial flags and protocol */
 	first->tx_flags = tx_flags;
 	first->protocol = protocol;
@@ -8548,11 +8576,11 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
 	}
 
 #endif /* IXGBE_FCOE */
-	tso = ixgbe_tso(tx_ring, first, &hdr_len);
+	tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx);
 	if (tso < 0)
 		goto out_drop;
 	else if (!tso)
-		ixgbe_tx_csum(tx_ring, first);
+		ixgbe_tx_csum(tx_ring, first, &ipsec_tx);
 
 	/* add the ATR filter if ATR is on */
 	if (test_bit(__IXGBE_TX_FDIR_INIT_DONE, &tx_ring->state))
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 8/9] ixgbe: add the Rx ipsec offload processing

[Shannon Nelson]

Add the ipsec check and processing into the Rx handler.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a434675..2bb4c5f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2424,6 +2424,9 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 			}
 		}
 #endif /* IXGBE_FCOE */
+		/* was IPsec offload processing done?  */
+		if (adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)
+			ixgbe_ipsec_process_rx_offload(rx_ring, rx_desc, skb);
 
 		ixgbe_rx_skb(q_vector, skb);
 
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 9/9] ixgbe: add init and remove ipsec support

[Shannon Nelson]

Link in the ipsec initialization and removal code, as well as the
table restore after a device reset.  This registers itself with
the XFRM subsystem and enables the driver's offload processing.

With this the ipsec offload support should be operational.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2bb4c5f..35f2d2a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5483,6 +5483,7 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter)
 
 	ixgbe_set_rx_mode(adapter->netdev);
 	ixgbe_restore_vlan(adapter);
+	ixgbe_ipsec_restore(adapter);
 
 	switch (hw->mac.type) {
 	case ixgbe_mac_82599EB:
@@ -6171,6 +6172,9 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
 	adapter->fcoe.up = IXGBE_FCOE_DEFTC;
 #endif /* CONFIG_IXGBE_DCB */
 #endif /* IXGBE_FCOE */
+#ifdef CONFIG_XFRM_OFFLOAD
+	adapter->flags2 |= IXGBE_FLAG2_IPSEC_CAPABLE;
+#endif /* CONFIG_XFRM_OFFLOAD */
 
 	/* initialize static ixgbe jump table entries */
 	adapter->jump_tables[0] = kzalloc(sizeof(*adapter->jump_tables[0]),
@@ -10543,6 +10547,9 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 #endif /* IXGBE_FCOE */
 
+	if (adapter->flags2 & IXGBE_FLAG2_IPSEC_CAPABLE)
+		ixgbe_init_ipsec_offload(adapter);
+
 	if (adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE)
 		netdev->hw_features |= NETIF_F_LRO;
 	if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
@@ -10755,6 +10762,11 @@ static void ixgbe_remove(struct pci_dev *pdev)
 	set_bit(__IXGBE_REMOVING, &adapter->state);
 	cancel_work_sync(&adapter->service_task);
 
+#ifdef CONFIG_XFRM_OFFLOAD
+	kfree(adapter->rx_ip_sa_tbl);
+	kfree(adapter->rx_sa_tbl);
+	kfree(adapter->tx_sa_tbl);
+#endif /* CONFIG_XFRM_OFFLOAD */
 
 #ifdef CONFIG_IXGBE_DCA
 	if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
-- 
2.7.4

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 1/9] ixgbe: remove undefined and conflicting bits

[Alexander Duyck]

On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> The PE and OSE error bits from the Advanced Receive Writeback
> descriptor are undefined in the published Niantic family datasheets
> and conflict with the defined SECERR bits, which we will need in
> a coming patch.
>
> The CE and LE error bits are also marked as reserved in the
> datasheet, but I'll let someone else remove them.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index ffa0ee5..0a77941 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -2375,8 +2375,6 @@ enum {
>  #define IXGBE_RXDADV_ERR_HBO    0x00800000 /*Header Buffer Overflow */
>  #define IXGBE_RXDADV_ERR_CE     0x01000000 /* CRC Error */
>  #define IXGBE_RXDADV_ERR_LE     0x02000000 /* Length Error */
> -#define IXGBE_RXDADV_ERR_PE     0x08000000 /* Packet Error */
> -#define IXGBE_RXDADV_ERR_OSE    0x10000000 /* Oversize Error */
>  #define IXGBE_RXDADV_ERR_USE    0x20000000 /* Undersize Error */
>  #define IXGBE_RXDADV_ERR_TCPE   0x40000000 /* TCP/UDP Checksum Error */
>  #define IXGBE_RXDADV_ERR_IPE    0x80000000 /* IP Checksum Error */
> @@ -2482,8 +2480,6 @@ enum {
>  #define IXGBE_RXDADV_ERR_FRAME_ERR_MASK ( \
>                                       IXGBE_RXDADV_ERR_CE | \
>                                       IXGBE_RXDADV_ERR_LE | \
> -                                     IXGBE_RXDADV_ERR_PE | \
> -                                     IXGBE_RXDADV_ERR_OSE | \
>                                       IXGBE_RXDADV_ERR_USE)
>
>  /* Multicast bit mask */

So I have to NAK this. Those bits do apply to 82598, and were
redefined for 82599 and later.

I'm assuming int he later patches I will see the reason why you needed
to drop them, but for now you might look at splitting the mask up and
leaving this as-is for 82598 and then dropping those two bits for
82599 if we need to.

- Alex

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 2/9] ixgbe: clean up ipsec defines

[Alexander Duyck]

On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> Clean up the ipsec/macsec descriptor bit definitions to match the rest
> of the defines and file organization.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index 0a77941..befed8b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -2321,11 +2321,6 @@ enum {
>  #define IXGBE_TXD_CMD_VLE    0x40000000 /* Add VLAN tag */
>  #define IXGBE_TXD_STAT_DD    0x00000001 /* Descriptor Done */
>
> -#define IXGBE_RXDADV_IPSEC_STATUS_SECP                  0x00020000
> -#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_PROTOCOL       0x08000000
> -#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_LENGTH         0x10000000
> -#define IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED            0x18000000
> -#define IXGBE_RXDADV_IPSEC_ERROR_BIT_MASK               0x18000000
>  /* Multiple Transmit Queue Command Register */
>  #define IXGBE_MTQC_RT_ENA       0x1 /* DCB Enable */
>  #define IXGBE_MTQC_VT_ENA       0x2 /* VMDQ2 Enable */
> @@ -2375,6 +2370,14 @@ enum {
>  #define IXGBE_RXDADV_ERR_HBO    0x00800000 /*Header Buffer Overflow */
>  #define IXGBE_RXDADV_ERR_CE     0x01000000 /* CRC Error */
>  #define IXGBE_RXDADV_ERR_LE     0x02000000 /* Length Error */
> +
> +#define IXGBE_RXDADV_ERR_MACSEC_NO_SA_MATCH  0x08000000
> +#define IXGBE_RXDADV_ERR_MACSEC_REPLAY_ERROR 0x10000000
> +#define IXGBE_RXDADV_ERR_MACSEC_BAD_SIG      0x18000000

I assume you are going to need the ipsec defines, what is the reason
fro adding the macsec ones? If they aren't needed I would say just
drop them as we don't need to be carrying them in the code if they
aren't going to be used.

> +#define IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL  0x08000000
> +#define IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH    0x10000000
> +#define IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED   0x18000000
> +
>  #define IXGBE_RXDADV_ERR_USE    0x20000000 /* Undersize Error */
>  #define IXGBE_RXDADV_ERR_TCPE   0x40000000 /* TCP/UDP Checksum Error */
>  #define IXGBE_RXDADV_ERR_IPE    0x80000000 /* IP Checksum Error */
> @@ -2396,6 +2399,7 @@ enum {
>  #define IXGBE_RXDADV_STAT_FCSTAT_FCPRSP 0x00000020 /* 10: Recv. FCP_RSP */
>  #define IXGBE_RXDADV_STAT_FCSTAT_DDP    0x00000030 /* 11: Ctxt w/ DDP */
>  #define IXGBE_RXDADV_STAT_TS           0x00010000 /* IEEE 1588 Time Stamp */
> +#define IXGBE_RXDADV_STAT_SECP          0x00020000 /* IPsec/MACsec pkt found */
>
>  /* PSRTYPE bit definitions */
>  #define IXGBE_PSRTYPE_TCPHDR    0x00000010
> @@ -2462,13 +2466,6 @@ enum {
>  #define IXGBE_RXDADV_PKTTYPE_ETQF_MASK  0x00000070 /* ETQF has 8 indices */
>  #define IXGBE_RXDADV_PKTTYPE_ETQF_SHIFT 4          /* Right-shift 4 bits */
>
> -/* Security Processing bit Indication */
> -#define IXGBE_RXDADV_LNKSEC_STATUS_SECP         0x00020000
> -#define IXGBE_RXDADV_LNKSEC_ERROR_NO_SA_MATCH   0x08000000
> -#define IXGBE_RXDADV_LNKSEC_ERROR_REPLAY_ERROR  0x10000000
> -#define IXGBE_RXDADV_LNKSEC_ERROR_BIT_MASK      0x18000000
> -#define IXGBE_RXDADV_LNKSEC_ERROR_BAD_SIG       0x18000000
> -
>  /* Masks to determine if packets should be dropped due to frame errors */
>  #define IXGBE_RXD_ERR_FRAME_ERR_MASK ( \
>                                       IXGBE_RXD_ERR_CE | \
> @@ -2889,6 +2886,7 @@ struct ixgbe_adv_tx_context_desc {
>                                  IXGBE_ADVTXD_POPTS_SHIFT)
>  #define IXGBE_ADVTXD_POPTS_TXSM (IXGBE_TXD_POPTS_TXSM << \
>                                  IXGBE_ADVTXD_POPTS_SHIFT)
> +#define IXGBE_ADVTXD_POPTS_IPSEC     0x00000400 /* IPSec offload request */
>  #define IXGBE_ADVTXD_POPTS_ISCO_1ST  0x00000000 /* 1st TSO of iSCSI PDU */
>  #define IXGBE_ADVTXD_POPTS_ISCO_MDL  0x00000800 /* Middle TSO of iSCSI PDU */
>  #define IXGBE_ADVTXD_POPTS_ISCO_LAST 0x00001000 /* Last TSO of iSCSI PDU */
> @@ -2904,7 +2902,6 @@ struct ixgbe_adv_tx_context_desc {
>  #define IXGBE_ADVTXD_TUCMD_L4T_SCTP  0x00001000  /* L4 Packet TYPE of SCTP */
>  #define IXGBE_ADVTXD_TUCMD_L4T_RSV     0x00001800 /* RSV L4 Packet TYPE */
>  #define IXGBE_ADVTXD_TUCMD_MKRREQ    0x00002000 /*Req requires Markers and CRC*/
> -#define IXGBE_ADVTXD_POPTS_IPSEC      0x00000400 /* IPSec offload request */
>  #define IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP 0x00002000 /* IPSec Type ESP */
>  #define IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN 0x00004000/* ESP Encrypt Enable */
>  #define IXGBE_ADVTXT_TUCMD_FCOE      0x00008000       /* FCoE Frame Type */
> --
> 2.7.4
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 6/9] ixgbe: add ipsec debug helpers

[Alexander Duyck]

On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> These are a couple of debugfs commands to inspect and manage the ipsec
> tables.  I really don't expect this patch to be in the final patchset.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
> index 5e2c1e3..0d0a9f0 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
> @@ -120,11 +120,27 @@ static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
>                 } else {
>                         e_dev_info("read <reg>\n");
>                 }
> +#ifdef CONFIG_XFRM_OFFLOAD
> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-sw-dump", 13) == 0) {
> +               ixgbe_ipsec_dump_info(adapter);
> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-hw-dump", 13) == 0) {
> +               ixgbe_ipsec_dump_hw(adapter);
> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-hw-clean", 14) == 0) {
> +               ixgbe_ipsec_clear_hw_tables(adapter);
> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-restore", 13) == 0) {
> +               ixgbe_ipsec_restore(adapter);
> +#endif /* CONFIG_XFRM_OFFLOAD */
>         } else {
>                 e_dev_info("Unknown command %s\n", ixgbe_dbg_reg_ops_buf);
>                 e_dev_info("Available commands:\n");
>                 e_dev_info("   read <reg>\n");
>                 e_dev_info("   write <reg> <value>\n");
> +#ifdef CONFIG_XFRM_OFFLOAD
> +               e_dev_info("   ipsec-sw-dump\n");
> +               e_dev_info("   ipsec-hw-dump\n");
> +               e_dev_info("   ipsec-hw-clean\n");
> +               e_dev_info("   ipsec-restore\n");
> +#endif /* CONFIG_XFRM_OFFLOAD */
>         }
>         return count;
>  }

NAK. Please don't use debugfs as any sort of configuration interface.
The clean and restore are very unlikely to ever be accepted upstream.

I'm not even sure if the dumping functionality is a good idea since it
seems like it might be a way to expose critical information for a
connection that is supposed to be secured via ipsec.

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 7/9] ixgbe: add tx ipsec offload

[Alexander Duyck]

On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> Add the Tx offload mechanisms.
>
> Currently this is slightly broken and not setting up the Tx checksum
> offload correctly, so is best used with
>         ethtool -K ethN tx off
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>

So the one piece that really seems to be missing here is any update to
ixgbe_features_check. That seems like it should be an integral part of
all this. With that you would be able to disable features like
checksum and TSO until you know how they would be configured and you
can push the checksum offload and segmentation up into the stack until
you can sort out how you do both the ipsec offload and those other
offloads.

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 44 ++++++++++++++++++++++-----
>  1 file changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 7f503d3..a434675 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -2423,8 +2423,8 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
>                                 continue;
>                         }
>                 }
> -
>  #endif /* IXGBE_FCOE */
> +
>                 ixgbe_rx_skb(q_vector, skb);
>
>                 /* update budget accounting */

This is white space noise and can be dropped from the patch.

> @@ -7791,9 +7791,10 @@ static void ixgbe_service_task(struct work_struct *work)
>
>  static int ixgbe_tso(struct ixgbe_ring *tx_ring,
>                      struct ixgbe_tx_buffer *first,
> -                    u8 *hdr_len)
> +                    u8 *hdr_len,
> +                    struct ixgbe_ipsec_tx_data *itd)
>  {
> -       u32 vlan_macip_lens, type_tucmd, mss_l4len_idx;
> +       u32 vlan_macip_lens, type_tucmd, mss_l4len_idx, fcoe_sof_eof = 0;

I realize that fcoe_sof_eof is probably what we were calling that part
of the context descriptor, but you may want to choose a name that
better describes how we are using this field.

>         struct sk_buff *skb = first->skb;
>         union {
>                 struct iphdr *v4;
> @@ -7872,7 +7873,14 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
>         vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT;
>         vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
>
> -       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd,
> +#ifdef CONFIG_XFRM_OFFLOAD
> +       if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
> +               fcoe_sof_eof |= itd->sa_idx;
> +               type_tucmd |= itd->flags | itd->trailer_len;
> +       }

Reading the datasheet for the X550, as per section 7.13.4.3 it looks
like these parts expect no trailer to be added to the packet in the
case of TSO. Do you know if the trailer is provided with a packet
being sent for TSO? If so it needs to be stripped, and it will be
added by hardware.

> +#endif /* CONFIG_XFRM_OFFLOAD */
> +
> +       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fcoe_sof_eof, type_tucmd,
>                           mss_l4len_idx);
>
>         return 1;



> @@ -7888,10 +7896,12 @@ static inline bool ixgbe_ipv6_csum_is_sctp(struct sk_buff *skb)
>  }
>
>  static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
> -                         struct ixgbe_tx_buffer *first)
> +                         struct ixgbe_tx_buffer *first,
> +                         struct ixgbe_ipsec_tx_data *itd)
>  {
>         struct sk_buff *skb = first->skb;
>         u32 vlan_macip_lens = 0;
> +       u32 fcoe_sof_eof = 0;
>         u32 type_tucmd = 0;
>
>         if (skb->ip_summed != CHECKSUM_PARTIAL) {
> @@ -7932,7 +7942,15 @@ static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
>         vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT;
>         vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
>
> -       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, 0);
> +#ifdef CONFIG_XFRM_OFFLOAD
> +       if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
> +               fcoe_sof_eof |= itd->sa_idx;
> +               type_tucmd |= itd->flags | itd->trailer_len;
> +       }
> +#endif /* CONFIG_XFRM_OFFLOAD */
> +
> +       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens,
> +                         fcoe_sof_eof, type_tucmd, 0);
>  }
>

So I am not sure what flags you would need to be adding to the
type_tucmd portion of the context descriptor. From what I can tell it
should just be the trailer length that gets dropped in there.

>  #define IXGBE_SET_FLAG(_input, _flag, _result) \
> @@ -7980,6 +7998,11 @@ static void ixgbe_tx_olinfo_status(union ixgbe_adv_tx_desc *tx_desc,
>                                         IXGBE_TX_FLAGS_IPV4,
>                                         IXGBE_ADVTXD_POPTS_IXSM);
>
> +       /* enble IPsec */
> +       olinfo_status |= IXGBE_SET_FLAG(tx_flags,
> +                                       IXGBE_TX_FLAGS_IPSEC,
> +                                       IXGBE_ADVTXD_POPTS_IPSEC);
> +
>         /*
>          * Check Context must be set if Tx switch is enabled, which it
>          * always is for case where virtual functions are running
> @@ -8444,6 +8467,7 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>         u32 tx_flags = 0;
>         unsigned short f;
>         u16 count = TXD_USE_COUNT(skb_headlen(skb));
> +       struct ixgbe_ipsec_tx_data ipsec_tx;
>         __be16 protocol = skb->protocol;
>         u8 hdr_len = 0;
>
> @@ -8532,6 +8556,10 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>                 }
>         }
>
> +       if ((adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED) &&
> +           ixgbe_ipsec_tx(tx_ring, skb, &ipsec_tx))
> +               tx_flags |= IXGBE_TX_FLAGS_IPSEC | IXGBE_TX_FLAGS_CC;
> +
>         /* record initial flags and protocol */
>         first->tx_flags = tx_flags;
>         first->protocol = protocol;
> @@ -8548,11 +8576,11 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>         }
>
>  #endif /* IXGBE_FCOE */
> -       tso = ixgbe_tso(tx_ring, first, &hdr_len);
> +       tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx);
>         if (tso < 0)
>                 goto out_drop;
>         else if (!tso)
> -               ixgbe_tx_csum(tx_ring, first);
> +               ixgbe_tx_csum(tx_ring, first, &ipsec_tx);
>
>         /* add the ATR filter if ATR is on */
>         if (test_bit(__IXGBE_TX_FDIR_INIT_DONE, &tx_ring->state))

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 8/9] ixgbe: add the Rx ipsec offload processing

[Alexander Duyck]

On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> Add the ipsec check and processing into the Rx handler.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index a434675..2bb4c5f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -2424,6 +2424,9 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
>                         }
>                 }
>  #endif /* IXGBE_FCOE */
> +               /* was IPsec offload processing done?  */
> +               if (adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)
> +                       ixgbe_ipsec_process_rx_offload(rx_ring, rx_desc, skb);
>
>                 ixgbe_rx_skb(q_vector, skb);

Generally I prefer to use ring flags rather than global ones. It keeps
us from having to reach too far. In addition you could probably make
this a part of ixgbe_process_skb_fields since that is the proper place
for handling anything that updates metadata in the skb.

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 1/9] ixgbe: remove undefined and conflicting bits

[Shannon Nelson]

On 11/17/2017 8:58 AM, Alexander Duyck wrote:
> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> The PE and OSE error bits from the Advanced Receive Writeback
>> descriptor are undefined in the published Niantic family datasheets
>> and conflict with the defined SECERR bits, which we will need in
>> a coming patch.
>>
>> The CE and LE error bits are also marked as reserved in the
>> datasheet, but I'll let someone else remove them.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> index ffa0ee5..0a77941 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> @@ -2375,8 +2375,6 @@ enum {
>>   #define IXGBE_RXDADV_ERR_HBO    0x00800000 /*Header Buffer Overflow */
>>   #define IXGBE_RXDADV_ERR_CE     0x01000000 /* CRC Error */
>>   #define IXGBE_RXDADV_ERR_LE     0x02000000 /* Length Error */
>> -#define IXGBE_RXDADV_ERR_PE     0x08000000 /* Packet Error */
>> -#define IXGBE_RXDADV_ERR_OSE    0x10000000 /* Oversize Error */
>>   #define IXGBE_RXDADV_ERR_USE    0x20000000 /* Undersize Error */
>>   #define IXGBE_RXDADV_ERR_TCPE   0x40000000 /* TCP/UDP Checksum Error */
>>   #define IXGBE_RXDADV_ERR_IPE    0x80000000 /* IP Checksum Error */
>> @@ -2482,8 +2480,6 @@ enum {
>>   #define IXGBE_RXDADV_ERR_FRAME_ERR_MASK ( \
>>                                        IXGBE_RXDADV_ERR_CE | \
>>                                        IXGBE_RXDADV_ERR_LE | \
>> -                                     IXGBE_RXDADV_ERR_PE | \
>> -                                     IXGBE_RXDADV_ERR_OSE | \
>>                                        IXGBE_RXDADV_ERR_USE)
>>
>>   /* Multicast bit mask */
> 
> So I have to NAK this. Those bits do apply to 82598, and were
> redefined for 82599 and later.
> 
> I'm assuming int he later patches I will see the reason why you needed
> to drop them, but for now you might look at splitting the mask up and
> leaving this as-is for 82598 and then dropping those two bits for
> 82599 if we need to.

The reason is not necessarily clear from the patches, but this macro is 
used in ixgbe_cleanup_headers() to check for errors and subsequently 
throw away the packets if any of these bits are seen.  This means that 
any packets with ipsec errors are thrown away before the ipsec code sees 
them.

As it turns out, the XFRM code further up in the stack really doesn't 
want the broken packets anyway, so we might as well throw them out, tho' 
it might be nice to track some statistics about bad ipsec packets.

I can leave the multiply-defined bits and add some commentary on the 
macro definition that this is going to catch IPsec error bits as well.

sln

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 2/9] ixgbe: clean up ipsec defines

[Shannon Nelson]

On 11/17/2017 9:02 AM, Alexander Duyck wrote:
> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> Clean up the ipsec/macsec descriptor bit definitions to match the rest
>> of the defines and file organization.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 23 ++++++++++-------------
>>   1 file changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> index 0a77941..befed8b 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> @@ -2321,11 +2321,6 @@ enum {
>>   #define IXGBE_TXD_CMD_VLE    0x40000000 /* Add VLAN tag */
>>   #define IXGBE_TXD_STAT_DD    0x00000001 /* Descriptor Done */
>>
>> -#define IXGBE_RXDADV_IPSEC_STATUS_SECP                  0x00020000
>> -#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_PROTOCOL       0x08000000
>> -#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_LENGTH         0x10000000
>> -#define IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED            0x18000000
>> -#define IXGBE_RXDADV_IPSEC_ERROR_BIT_MASK               0x18000000
>>   /* Multiple Transmit Queue Command Register */
>>   #define IXGBE_MTQC_RT_ENA       0x1 /* DCB Enable */
>>   #define IXGBE_MTQC_VT_ENA       0x2 /* VMDQ2 Enable */
>> @@ -2375,6 +2370,14 @@ enum {
>>   #define IXGBE_RXDADV_ERR_HBO    0x00800000 /*Header Buffer Overflow */
>>   #define IXGBE_RXDADV_ERR_CE     0x01000000 /* CRC Error */
>>   #define IXGBE_RXDADV_ERR_LE     0x02000000 /* Length Error */
>> +
>> +#define IXGBE_RXDADV_ERR_MACSEC_NO_SA_MATCH  0x08000000
>> +#define IXGBE_RXDADV_ERR_MACSEC_REPLAY_ERROR 0x10000000
>> +#define IXGBE_RXDADV_ERR_MACSEC_BAD_SIG      0x18000000
> 
> I assume you are going to need the ipsec defines, what is the reason
> fro adding the macsec ones? If they aren't needed I would say just
> drop them as we don't need to be carrying them in the code if they
> aren't going to be used.

I didn't add them but changed their names from the unused LNKSEC defines 
a few lines further down, and kept them around for future travelers.  I 
don't have a problem dropping them.

sln

> 
>> +#define IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL  0x08000000
>> +#define IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH    0x10000000
>> +#define IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED   0x18000000
>> +
>>   #define IXGBE_RXDADV_ERR_USE    0x20000000 /* Undersize Error */
>>   #define IXGBE_RXDADV_ERR_TCPE   0x40000000 /* TCP/UDP Checksum Error */
>>   #define IXGBE_RXDADV_ERR_IPE    0x80000000 /* IP Checksum Error */
>> @@ -2396,6 +2399,7 @@ enum {
>>   #define IXGBE_RXDADV_STAT_FCSTAT_FCPRSP 0x00000020 /* 10: Recv. FCP_RSP */
>>   #define IXGBE_RXDADV_STAT_FCSTAT_DDP    0x00000030 /* 11: Ctxt w/ DDP */
>>   #define IXGBE_RXDADV_STAT_TS           0x00010000 /* IEEE 1588 Time Stamp */
>> +#define IXGBE_RXDADV_STAT_SECP          0x00020000 /* IPsec/MACsec pkt found */
>>
>>   /* PSRTYPE bit definitions */
>>   #define IXGBE_PSRTYPE_TCPHDR    0x00000010
>> @@ -2462,13 +2466,6 @@ enum {
>>   #define IXGBE_RXDADV_PKTTYPE_ETQF_MASK  0x00000070 /* ETQF has 8 indices */
>>   #define IXGBE_RXDADV_PKTTYPE_ETQF_SHIFT 4          /* Right-shift 4 bits */
>>
>> -/* Security Processing bit Indication */
>> -#define IXGBE_RXDADV_LNKSEC_STATUS_SECP         0x00020000
>> -#define IXGBE_RXDADV_LNKSEC_ERROR_NO_SA_MATCH   0x08000000
>> -#define IXGBE_RXDADV_LNKSEC_ERROR_REPLAY_ERROR  0x10000000
>> -#define IXGBE_RXDADV_LNKSEC_ERROR_BIT_MASK      0x18000000
>> -#define IXGBE_RXDADV_LNKSEC_ERROR_BAD_SIG       0x18000000
>> -
>>   /* Masks to determine if packets should be dropped due to frame errors */
>>   #define IXGBE_RXD_ERR_FRAME_ERR_MASK ( \
>>                                        IXGBE_RXD_ERR_CE | \
>> @@ -2889,6 +2886,7 @@ struct ixgbe_adv_tx_context_desc {
>>                                   IXGBE_ADVTXD_POPTS_SHIFT)
>>   #define IXGBE_ADVTXD_POPTS_TXSM (IXGBE_TXD_POPTS_TXSM << \
>>                                   IXGBE_ADVTXD_POPTS_SHIFT)
>> +#define IXGBE_ADVTXD_POPTS_IPSEC     0x00000400 /* IPSec offload request */
>>   #define IXGBE_ADVTXD_POPTS_ISCO_1ST  0x00000000 /* 1st TSO of iSCSI PDU */
>>   #define IXGBE_ADVTXD_POPTS_ISCO_MDL  0x00000800 /* Middle TSO of iSCSI PDU */
>>   #define IXGBE_ADVTXD_POPTS_ISCO_LAST 0x00001000 /* Last TSO of iSCSI PDU */
>> @@ -2904,7 +2902,6 @@ struct ixgbe_adv_tx_context_desc {
>>   #define IXGBE_ADVTXD_TUCMD_L4T_SCTP  0x00001000  /* L4 Packet TYPE of SCTP */
>>   #define IXGBE_ADVTXD_TUCMD_L4T_RSV     0x00001800 /* RSV L4 Packet TYPE */
>>   #define IXGBE_ADVTXD_TUCMD_MKRREQ    0x00002000 /*Req requires Markers and CRC*/
>> -#define IXGBE_ADVTXD_POPTS_IPSEC      0x00000400 /* IPSec offload request */
>>   #define IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP 0x00002000 /* IPSec Type ESP */
>>   #define IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN 0x00004000/* ESP Encrypt Enable */
>>   #define IXGBE_ADVTXT_TUCMD_FCOE      0x00008000       /* FCoE Frame Type */
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan at osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 6/9] ixgbe: add ipsec debug helpers

[Shannon Nelson]

On 11/17/2017 9:09 AM, Alexander Duyck wrote:
> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> These are a couple of debugfs commands to inspect and manage the ipsec
>> tables.  I really don't expect this patch to be in the final patchset.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
>> index 5e2c1e3..0d0a9f0 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
>> @@ -120,11 +120,27 @@ static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
>>                  } else {
>>                          e_dev_info("read <reg>\n");
>>                  }
>> +#ifdef CONFIG_XFRM_OFFLOAD
>> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-sw-dump", 13) == 0) {
>> +               ixgbe_ipsec_dump_info(adapter);
>> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-hw-dump", 13) == 0) {
>> +               ixgbe_ipsec_dump_hw(adapter);
>> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-hw-clean", 14) == 0) {
>> +               ixgbe_ipsec_clear_hw_tables(adapter);
>> +       } else if (strncmp(ixgbe_dbg_reg_ops_buf, "ipsec-restore", 13) == 0) {
>> +               ixgbe_ipsec_restore(adapter);
>> +#endif /* CONFIG_XFRM_OFFLOAD */
>>          } else {
>>                  e_dev_info("Unknown command %s\n", ixgbe_dbg_reg_ops_buf);
>>                  e_dev_info("Available commands:\n");
>>                  e_dev_info("   read <reg>\n");
>>                  e_dev_info("   write <reg> <value>\n");
>> +#ifdef CONFIG_XFRM_OFFLOAD
>> +               e_dev_info("   ipsec-sw-dump\n");
>> +               e_dev_info("   ipsec-hw-dump\n");
>> +               e_dev_info("   ipsec-hw-clean\n");
>> +               e_dev_info("   ipsec-restore\n");
>> +#endif /* CONFIG_XFRM_OFFLOAD */
>>          }
>>          return count;
>>   }
> 
> NAK. Please don't use debugfs as any sort of configuration interface.
> The clean and restore are very unlikely to ever be accepted upstream.
> 
> I'm not even sure if the dumping functionality is a good idea since it
> seems like it might be a way to expose critical information for a
> connection that is supposed to be secured via ipsec.
> 

Hence the commit comment about not being in the final patchset.
sln

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 7/9] ixgbe: add tx ipsec offload

[Shannon Nelson]

On 11/17/2017 9:43 AM, Alexander Duyck wrote:
> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> Add the Tx offload mechanisms.
>>
>> Currently this is slightly broken and not setting up the Tx checksum
>> offload correctly, so is best used with
>>          ethtool -K ethN tx off
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> 
> So the one piece that really seems to be missing here is any update to
> ixgbe_features_check. That seems like it should be an integral part of
> all this. With that you would be able to disable features like
> checksum and TSO until you know how they would be configured and you
> can push the checksum offload and segmentation up into the stack until
> you can sort out how you do both the ipsec offload and those other
> offloads.

That would make sense.

> 
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 44 ++++++++++++++++++++++-----
>>   1 file changed, 36 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index 7f503d3..a434675 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -2423,8 +2423,8 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
>>                                  continue;
>>                          }
>>                  }
>> -
>>   #endif /* IXGBE_FCOE */
>> +
>>                  ixgbe_rx_skb(q_vector, skb);
>>
>>                  /* update budget accounting */
> 
> This is white space noise and can be dropped from the patch.
> 
>> @@ -7791,9 +7791,10 @@ static void ixgbe_service_task(struct work_struct *work)
>>
>>   static int ixgbe_tso(struct ixgbe_ring *tx_ring,
>>                       struct ixgbe_tx_buffer *first,
>> -                    u8 *hdr_len)
>> +                    u8 *hdr_len,
>> +                    struct ixgbe_ipsec_tx_data *itd)
>>   {
>> -       u32 vlan_macip_lens, type_tucmd, mss_l4len_idx;
>> +       u32 vlan_macip_lens, type_tucmd, mss_l4len_idx, fcoe_sof_eof = 0;
> 
> I realize that fcoe_sof_eof is probably what we were calling that part
> of the context descriptor, but you may want to choose a name that
> better describes how we are using this field.

I just figured to name it in keeping with the parameter name in 
ixgbe_tx_ctxtdesc().  Perhaps changing it in both places to something 
like fcoef_saidx would better match the datasheet.  Of course, since 
it's a multifaceted and useful field, maybe I'll name it "Mitch" :-).

Then there's the context descriptor field name itself: seqnum_seed. 
Where did that come from?  I'm tempted to rename that thing too.


> 
>>          struct sk_buff *skb = first->skb;
>>          union {
>>                  struct iphdr *v4;
>> @@ -7872,7 +7873,14 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
>>          vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT;
>>          vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
>>
>> -       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd,
>> +#ifdef CONFIG_XFRM_OFFLOAD
>> +       if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
>> +               fcoe_sof_eof |= itd->sa_idx;
>> +               type_tucmd |= itd->flags | itd->trailer_len;
>> +       }
> 
> Reading the datasheet for the X550, as per section 7.13.4.3 it looks
> like these parts expect no trailer to be added to the packet in the
> case of TSO. Do you know if the trailer is provided with a packet
> being sent for TSO? If so it needs to be stripped, and it will be
> added by hardware.

Yep, I think you're right.

> 
>> +#endif /* CONFIG_XFRM_OFFLOAD */
>> +
>> +       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fcoe_sof_eof, type_tucmd,
>>                            mss_l4len_idx);
>>
>>          return 1;
> 
> 
> 
>> @@ -7888,10 +7896,12 @@ static inline bool ixgbe_ipv6_csum_is_sctp(struct sk_buff *skb)
>>   }
>>
>>   static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
>> -                         struct ixgbe_tx_buffer *first)
>> +                         struct ixgbe_tx_buffer *first,
>> +                         struct ixgbe_ipsec_tx_data *itd)
>>   {
>>          struct sk_buff *skb = first->skb;
>>          u32 vlan_macip_lens = 0;
>> +       u32 fcoe_sof_eof = 0;
>>          u32 type_tucmd = 0;
>>
>>          if (skb->ip_summed != CHECKSUM_PARTIAL) {
>> @@ -7932,7 +7942,15 @@ static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
>>          vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT;
>>          vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
>>
>> -       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, 0);
>> +#ifdef CONFIG_XFRM_OFFLOAD
>> +       if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
>> +               fcoe_sof_eof |= itd->sa_idx;
>> +               type_tucmd |= itd->flags | itd->trailer_len;
>> +       }
>> +#endif /* CONFIG_XFRM_OFFLOAD */
>> +
>> +       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens,
>> +                         fcoe_sof_eof, type_tucmd, 0);
>>   }
>>
> 
> So I am not sure what flags you would need to be adding to the
> type_tucmd portion of the context descriptor. From what I can tell it
> should just be the trailer length that gets dropped in there.

That's correct.

sln

> 
>>   #define IXGBE_SET_FLAG(_input, _flag, _result) \
>> @@ -7980,6 +7998,11 @@ static void ixgbe_tx_olinfo_status(union ixgbe_adv_tx_desc *tx_desc,
>>                                          IXGBE_TX_FLAGS_IPV4,
>>                                          IXGBE_ADVTXD_POPTS_IXSM);
>>
>> +       /* enble IPsec */
>> +       olinfo_status |= IXGBE_SET_FLAG(tx_flags,
>> +                                       IXGBE_TX_FLAGS_IPSEC,
>> +                                       IXGBE_ADVTXD_POPTS_IPSEC);
>> +
>>          /*
>>           * Check Context must be set if Tx switch is enabled, which it
>>           * always is for case where virtual functions are running
>> @@ -8444,6 +8467,7 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>>          u32 tx_flags = 0;
>>          unsigned short f;
>>          u16 count = TXD_USE_COUNT(skb_headlen(skb));
>> +       struct ixgbe_ipsec_tx_data ipsec_tx;
>>          __be16 protocol = skb->protocol;
>>          u8 hdr_len = 0;
>>
>> @@ -8532,6 +8556,10 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>>                  }
>>          }
>>
>> +       if ((adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED) &&
>> +           ixgbe_ipsec_tx(tx_ring, skb, &ipsec_tx))
>> +               tx_flags |= IXGBE_TX_FLAGS_IPSEC | IXGBE_TX_FLAGS_CC;
>> +
>>          /* record initial flags and protocol */
>>          first->tx_flags = tx_flags;
>>          first->protocol = protocol;
>> @@ -8548,11 +8576,11 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>>          }
>>
>>   #endif /* IXGBE_FCOE */
>> -       tso = ixgbe_tso(tx_ring, first, &hdr_len);
>> +       tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx);
>>          if (tso < 0)
>>                  goto out_drop;
>>          else if (!tso)
>> -               ixgbe_tx_csum(tx_ring, first);
>> +               ixgbe_tx_csum(tx_ring, first, &ipsec_tx);
>>
>>          /* add the ATR filter if ATR is on */
>>          if (test_bit(__IXGBE_TX_FDIR_INIT_DONE, &tx_ring->state))

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 8/9] ixgbe: add the Rx ipsec offload processing

[Shannon Nelson]

On 11/17/2017 9:45 AM, Alexander Duyck wrote:
> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> Add the ipsec check and processing into the Rx handler.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index a434675..2bb4c5f 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -2424,6 +2424,9 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
>>                          }
>>                  }
>>   #endif /* IXGBE_FCOE */
>> +               /* was IPsec offload processing done?  */
>> +               if (adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)
>> +                       ixgbe_ipsec_process_rx_offload(rx_ring, rx_desc, skb);
>>
>>                  ixgbe_rx_skb(q_vector, skb);
> 
> Generally I prefer to use ring flags rather than global ones. It keeps
> us from having to reach too far. In addition you could probably make
> this a part of ixgbe_process_skb_fields since that is the proper place
> for handling anything that updates metadata in the skb.

Both of those comments make sense.

sln

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 3/9] ixgbe: add the ipsec support code files

[Alexander Duyck]

On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> This adds the ipsec support code into the driver, but doesn't yet
> include it into the compile.
>
> It is possible that the .h file contents could simply go into
> the ixgbe_type.h and ixgbe.h files.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>

Generally I really hate it when code is just thrown in like this as it
is really difficult to review and/or debug. I would really much prefer
to have this code added as it is needed for specific functionality.

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 1133 ++++++++++++++++++++++++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h |   80 ++
>  2 files changed, 1213 insertions(+)
>  create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>  create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> new file mode 100644
> index 0000000..177b915
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -0,0 +1,1133 @@
> +/*******************************************************************************
> + *
> + * Intel 10 Gigabit PCI Express Linux driver
> + * Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * The full GNU General Public License is included in this distribution in
> + * the file called "COPYING".
> + *
> + * Contact Information:
> + * Linux NICS <linux.nics@intel.com>
> + * e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
> + * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
> + *
> + ******************************************************************************/
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/netdevice.h>
> +#include <linux/vmalloc.h>
> +#include <linux/string.h>
> +#include <linux/in.h>
> +#include <linux/ip.h>
> +#include <linux/tcp.h>
> +#include <linux/ipv6.h>
> +#include <net/xfrm.h>
> +#include <crypto/aead.h>
> +
> +#include "ixgbe.h"
> +#include "ixgbe_type.h"
> +#include "ixgbe_ipsec.h"
> +
> +/**
> + * ixgbe_ipsec_dump_info - dump in-memory SA tables for debugging
> + * @adapter: pointer to adapter structure
> + **/
> +void ixgbe_ipsec_dump_info(struct ixgbe_adapter *adapter)
> +{
> +       struct net_device *dev = adapter->netdev;
> +       int i;
> +
> +       netdev_info(dev, "IXGBE_IPSEC_INFO_DUMP\n");
> +
> +       netdev_info(dev, "  Tx SA table\n");
> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
> +               struct tx_sa *tsa = &adapter->tx_sa_tbl[i];
> +
> +               if (!tsa->used)
> +                       continue;
> +
> +               netdev_info(dev, " tx_sa_tbl[%i] key = 0x%08x %08x %08x %08x\n",
> +                           i, tsa->key[0], tsa->key[1],
> +                           tsa->key[2], tsa->key[3]);
> +               netdev_info(dev, "              salt = 0x%08x encrypt = %d\n",
> +                           tsa->salt, tsa->encrypt);
> +       }
> +
> +       netdev_info(dev, "  Rx SA table\n");
> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
> +               struct rx_sa *rsa = &adapter->rx_sa_tbl[i];
> +
> +               if (!rsa->used)
> +                       continue;
> +
> +               netdev_info(dev, "rx_sa_tbl[%i] ipaddr = 0x%08x %08x %08x %08x\n",
> +                           i, rsa->ipaddr[0], rsa->ipaddr[1],
> +                           rsa->ipaddr[2], rsa->ipaddr[3]);
> +               netdev_info(dev, "             spi = 0x%08x proto = 0x%x iptbl_ind = 0x%08x mode = 0x%04x\n",
> +                           rsa->xs->id.spi, rsa->xs->id.proto,
> +                           rsa->iptbl_ind, rsa->mode);
> +               netdev_info(dev, "             key = 0x%08x %08x %08x %08x\n",
> +                           rsa->key[0], rsa->key[1],
> +                           rsa->key[2], rsa->key[3]);
> +               netdev_info(dev, "             salt = 0x%08x decrypt = %d xs = %p\n",
> +                           rsa->salt, rsa->decrypt, rsa->xs);
> +       }
> +
> +       netdev_info(dev, "  Rx IP table\n");
> +       for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
> +               struct rx_ip_sa *ipsa = &adapter->rx_ip_sa_tbl[i];
> +
> +               if (!ipsa->used)
> +                       continue;
> +
> +               netdev_info(dev, "rx_ip_tbl[%i] ipaddr = 0x%08x %08x %08x %08x ref_cnt = 0x%08x\n",
> +                           i, ipsa->ipaddr[0], ipsa->ipaddr[1],
> +                           ipsa->ipaddr[2], ipsa->ipaddr[3],
> +                           ipsa->ref_cnt);
> +       }
> +
> +       netdev_info(dev, "  Offload counts\n");
> +       for (i = 0; i < adapter->num_rx_queues; i++) {
> +               struct ixgbe_ring *rx_ring = adapter->rx_ring[i];
> +
> +               if (rx_ring->rx_stats.ipsec_offloads ||
> +                   rx_ring->rx_stats.ipsec_offload_faileds) {
> +                       netdev_info(dev, "rx_ring[%i] ipsec_offloads = %llu ipsec_offload_faileds = %llu\n",
> +                                   i, rx_ring->rx_stats.ipsec_offloads,
> +                                   rx_ring->rx_stats.ipsec_offload_faileds);
> +               }
> +       }
> +}
> +
> +/**
> + * ixgbe_ipsec_dump_hw - dump the contents of the hw tables
> + * @adapter: pointer to adapter structure
> + **/
> +void ixgbe_ipsec_dump_hw(struct ixgbe_adapter *adapter)
> +{
> +       struct net_device *dev = adapter->netdev;
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       int t_idx, i;
> +       u32 reg;
> +
> +       netdev_info(dev, "IPsec HW\n");
> +
> +       netdev_info(dev, "SECRXCTRL=0x%08x  SECRXSTAT=0x%08x\n",
> +                   IXGBE_READ_REG(hw, IXGBE_SECRXCTRL),
> +                   IXGBE_READ_REG(hw, IXGBE_SECRXSTAT));
> +
> +       netdev_info(dev, "SECTXCTRL=0x%08x  SECTXSTAT=0x%08x, SECTXBUFFAF=0x%08x, SECTXMINIFG=0x%08x\n",
> +                   IXGBE_READ_REG(hw, IXGBE_SECTXCTRL),
> +                   IXGBE_READ_REG(hw, IXGBE_SECTXSTAT),
> +                   IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF),
> +                   IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG));
> +
> +       netdev_info(dev, "RxIdx=0x%08x  TxIdx=0x%08x\n",
> +                   IXGBE_READ_REG(hw, IXGBE_IPSRXIDX),
> +                   IXGBE_READ_REG(hw, IXGBE_IPSTXIDX));
> +
> +       /* Tx SA Table */
> +       netdev_info(dev, "Tx SA HW Table\n");
> +       for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_SA_COUNT; t_idx++) {
> +               u32 key[4];
> +               u32 salt;
> +               u32 anybit = 0;
> +
> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
> +               reg &= IXGBE_RXTXIDX_IPS_EN;
> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ;
> +               IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, reg);
> +               for (i = 0; i < 4; i++) {
> +                       key[i] = IXGBE_READ_REG(hw, IXGBE_IPSTXKEY(i));
> +                       anybit |= key[i];
> +               }
> +               salt = IXGBE_READ_REG(hw, IXGBE_IPSTXSALT);
> +               anybit |= salt;
> +
> +               if (anybit)
> +                       netdev_info(dev, "  SA[%d] key=0x%08x 0x%08x 0x%08x 0x%08x salt=0x%08x\n",
> +                                   t_idx, key[3], key[2], key[1], key[0],
> +                                   salt);
> +       }
> +
> +       /* Rx SA Table */
> +       netdev_info(dev, "Rx SA HW Table\n");
> +       for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_SA_COUNT; t_idx++) {
> +               u32 key[4];
> +               u32 ip_idx;
> +               __be32 spi;
> +               u32 salt;
> +               u32 mode;
> +               u32 anybit = 0;
> +
> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
> +               reg &= IXGBE_RXTXIDX_IPS_EN;
> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_SPI;
> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
> +               spi = IXGBE_READ_REG(hw, IXGBE_IPSRXSPI);
> +               ip_idx = IXGBE_READ_REG(hw, IXGBE_IPSRXIPIDX);
> +
> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
> +               reg &= IXGBE_RXTXIDX_IPS_EN;
> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_KEY;
> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
> +               for (i = 0; i < 4; i++) {
> +                       key[i] = IXGBE_READ_REG(hw, IXGBE_IPSRXKEY(i));
> +                       anybit |= key[i];
> +               }
> +               salt = IXGBE_READ_REG(hw, IXGBE_IPSRXSALT);
> +               mode = IXGBE_READ_REG(hw, IXGBE_IPSRXMOD);
> +
> +               anybit |= salt | mode | spi | ip_idx;
> +               if (anybit)
> +                       netdev_info(dev, "  SA[%d] spi=0x%08x key=0x%08x 0x%08x 0x%08x 0x%08x salt=0x%08x ipi=0x%08x mode=0x%08x\n",
> +                                   t_idx, spi, key[3], key[2], key[1], key[0],
> +                                   salt, ip_idx, mode);
> +       }
> +
> +       /* Rx IP Table */
> +       netdev_info(dev, "Rx IP HW Table\n");
> +       for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; t_idx++) {
> +               u32 ipaddr[4];
> +               u32 anybit = 0;
> +
> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
> +               reg &= IXGBE_RXTXIDX_IPS_EN;
> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_IP;
> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
> +               for (i = 0; i < 4; i++) {
> +                       ipaddr[i] = IXGBE_READ_REG(hw, IXGBE_IPSRXIPADDR(i));
> +                       anybit |= ipaddr[i];
> +               }
> +
> +               if (anybit)
> +                       netdev_info(dev, "  IP[%d] 0x%08x 0x%08x 0x%08x 0x%08x\n",
> +                                   t_idx, ipaddr[0], ipaddr[1],
> +                                   ipaddr[2], ipaddr[3]);
> +       }
> +}
> +

So all of this dumping code can go. I am pretty sure it exposes a
pretty big security issue since it makes it far to easy for someone to
get access to the keys and such for a given ipsec connection.

> +/**
> + * ixgbe_ipsec_set_tx_sa - set the Tx SA registers
> + * @hw: hw specific details
> + * @idx: register index to write
> + * @key: key byte array
> + * @salt: salt bytes
> + **/
> +static void ixgbe_ipsec_set_tx_sa(struct ixgbe_hw *hw, u16 idx,
> +                                 u32 key[], u32 salt)
> +{
> +       u32 reg;
> +       int i;
> +
> +       for (i = 0; i < 4; i++)
> +               IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(i), cpu_to_be32(key[3-i]));
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, cpu_to_be32(salt));
> +       IXGBE_WRITE_FLUSH(hw);
> +
> +       reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
> +       reg &= IXGBE_RXTXIDX_IPS_EN;
> +       reg |= idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, reg);
> +       IXGBE_WRITE_FLUSH(hw);
> +}
> +
> +/**
> + * ixgbe_ipsec_set_rx_item - set an Rx table item
> + * @hw: hw specific details
> + * @idx: register index to write
> + * @tbl: table selector
> + *
> + * Trigger the device to store into a particular Rx table the
> + * data that has already been loaded into the input register
> + **/
> +static void ixgbe_ipsec_set_rx_item(struct ixgbe_hw *hw, u16 idx, u32 tbl)
> +{
> +       u32 reg;
> +
> +       reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
> +       reg &= IXGBE_RXTXIDX_IPS_EN;
> +       reg |= tbl | idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
> +       IXGBE_WRITE_FLUSH(hw);
> +}
> +
> +/**
> + * ixgbe_ipsec_set_rx_sa - set up the register bits to save SA info
> + * @hw: hw specific details
> + * @idx: register index to write
> + * @spi: security parameter index
> + * @key: key byte array
> + * @salt: salt bytes
> + * @mode: rx decrypt control bits
> + * @ip_idx: index into IP table for related IP address
> + **/
> +static void ixgbe_ipsec_set_rx_sa(struct ixgbe_hw *hw, u16 idx, __be32 spi,
> +                                 u32 key[], u32 salt, u32 mode, u32 ip_idx)
> +{
> +       int i;
> +
> +       /* store the SPI (in bigendian) and IPidx */
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, spi);
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, ip_idx);
> +       IXGBE_WRITE_FLUSH(hw);
> +
> +       ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_SPI);
> +
> +       /* store the key, salt, and mode */
> +       for (i = 0; i < 4; i++)
> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(i), cpu_to_be32(key[3-i]));
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, cpu_to_be32(salt));
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, mode);
> +       IXGBE_WRITE_FLUSH(hw);
> +
> +       ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_KEY);
> +}
> +
> +/**
> + * ixgbe_ipsec_set_rx_ip - set up the register bits to save SA IP addr info
> + * @hw: hw specific details
> + * @idx: register index to write
> + * @addr: IP address byte array
> + **/
> +static void ixgbe_ipsec_set_rx_ip(struct ixgbe_hw *hw, u16 idx, u32 addr[])
> +{
> +       int i;
> +
> +       /* store the ip address */
> +       for (i = 0; i < 4; i++)
> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(i), addr[i]);
> +       IXGBE_WRITE_FLUSH(hw);
> +
> +       ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_IP);
> +}
> +
> +/**
> + * ixgbe_ipsec_clear_hw_tables - because some tables don't get cleared on reset
> + * @hw: hw specific details
> + **/
> +void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
> +{
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       u32 buf[4] = {0, 0, 0, 0};
> +       u16 idx;
> +
> +       /* disable Rx and Tx SA lookup */
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
> +
> +       /* scrub the tables */
> +       for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
> +               ixgbe_ipsec_set_tx_sa(hw, idx, buf, 0);
> +       adapter->num_tx_sa = 0;
> +
> +       for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
> +               ixgbe_ipsec_set_rx_sa(hw, idx, 0, buf, 0, 0, 0);
> +       adapter->num_rx_sa = 0;
> +
> +       for (idx = 0; idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; idx++)
> +               ixgbe_ipsec_set_rx_ip(hw, idx, buf);
> +}
> +
> +/**
> + * ixgbe_ipsec_stop_data
> + * @hw: hw specific details
> + **/
> +static void ixgbe_ipsec_stop_data(struct ixgbe_adapter *adapter)
> +{
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       bool link = adapter->link_up;
> +       u32 t_rdy, r_rdy;
> +       u32 reg;
> +
> +       /* halt data paths */
> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
> +       reg |= IXGBE_SECTXCTRL_TX_DIS;
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
> +
> +       reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
> +       reg |= IXGBE_SECRXCTRL_RX_DIS;
> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
> +
> +       IXGBE_WRITE_FLUSH(hw);
> +
> +       /* If the tx fifo doesn't have link, but still has data,
> +        * we can't clear the tx sec block.  Set the MAC loopback
> +        * before block clear
> +        */
> +       if (!link) {
> +               reg = IXGBE_READ_REG(hw, IXGBE_MACC);
> +               reg |= IXGBE_MACC_FLU;
> +               IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
> +
> +               reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> +               reg |= IXGBE_HLREG0_LPBK;
> +               IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
> +
> +               IXGBE_WRITE_FLUSH(hw);
> +               mdelay(3);
> +       }
> +
> +       /* wait for the paths to empty */
> +       do {
> +               mdelay(10);
> +               t_rdy = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT) &
> +                       IXGBE_SECTXSTAT_SECTX_RDY;
> +               r_rdy = IXGBE_READ_REG(hw, IXGBE_SECRXSTAT) &
> +                       IXGBE_SECRXSTAT_SECRX_RDY;
> +       } while (!t_rdy && !r_rdy);
> +
> +       /* undo loopback if we played with it earlier */
> +       if (!link) {
> +               reg = IXGBE_READ_REG(hw, IXGBE_MACC);
> +               reg &= ~IXGBE_MACC_FLU;
> +               IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
> +
> +               reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> +               reg &= ~IXGBE_HLREG0_LPBK;
> +               IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
> +
> +               IXGBE_WRITE_FLUSH(hw);
> +       }
> +}
> +
> +/**
> + * ixgbe_ipsec_stop_engine
> + * @hw: hw specific details
> + **/
> +static void ixgbe_ipsec_stop_engine(struct ixgbe_adapter *adapter)
> +{
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       u32 reg;
> +
> +       ixgbe_ipsec_stop_data(adapter);
> +
> +       /* disable Rx and Tx SA lookup */
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
> +
> +       /* disable the Rx and Tx engines and full packet store-n-forward */
> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
> +       reg |= IXGBE_SECTXCTRL_SECTX_DIS;
> +       reg &= ~IXGBE_SECTXCTRL_STORE_FORWARD;
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
> +
> +       reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
> +       reg |= IXGBE_SECRXCTRL_SECRX_DIS;
> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
> +
> +       /* restore the "tx security buffer almost full threshold" to 0x250 */
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, 0x250);
> +
> +       /* Set minimum IFG between packets back to the default 0x1 */
> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
> +       reg = (reg & 0xfffffff0) | 0x1;
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
> +
> +       /* final set for normal (no ipsec offload) processing */
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_SECTX_DIS);
> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, IXGBE_SECRXCTRL_SECRX_DIS);
> +
> +       IXGBE_WRITE_FLUSH(hw);
> +}
> +
> +/**
> + * ixgbe_ipsec_start_engine
> + * @hw: hw specific details
> + *
> + * NOTE: this increases power consumption whether being used or not
> + **/
> +static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter)
> +{
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       u32 reg;
> +
> +       ixgbe_ipsec_stop_data(adapter);
> +
> +       /* Set minimum IFG between packets to 3 */
> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
> +       reg = (reg & 0xfffffff0) | 0x3;
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
> +
> +       /* Set "tx security buffer almost full threshold" to 0x15 so that the
> +        * almost full indication is generated only after buffer contains at
> +        * least an entire jumbo packet.
> +        */
> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF);
> +       reg = (reg & 0xfffffc00) | 0x15;
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, reg);
> +
> +       /* restart the data paths by clearing the DISABLE bits */
> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, 0);
> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_STORE_FORWARD);
> +
> +       /* enable Rx and Tx SA lookup */
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, IXGBE_RXTXIDX_IPS_EN);
> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, IXGBE_RXTXIDX_IPS_EN);
> +
> +       IXGBE_WRITE_FLUSH(hw);
> +}
> +
> +/**
> + * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset
> + * @hw: hw specific details
> + **/
> +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter)
> +{
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       u32 zbuf[4] = {0, 0, 0, 0};
> +       int i;
> +
> +       if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED))
> +               return;
> +
> +       /* clean up the engine settings */
> +       ixgbe_ipsec_stop_engine(adapter);
> +
> +       /* start the engine */
> +       ixgbe_ipsec_start_engine(adapter);
> +
> +       /* reload the IP addrs */
> +       for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
> +               struct rx_ip_sa *ipsa = &adapter->rx_ip_sa_tbl[i];
> +
> +               if (ipsa->used)
> +                       ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr);
> +               else
> +                       ixgbe_ipsec_set_rx_ip(hw, i, zbuf);
> +       }
> +
> +       /* reload the Rx keys */
> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
> +               struct rx_sa *rsa = &adapter->rx_sa_tbl[i];
> +
> +               if (rsa->used)
> +                       ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi,
> +                                             rsa->key, rsa->salt,
> +                                             rsa->mode, rsa->iptbl_ind);
> +               else
> +                       ixgbe_ipsec_set_rx_sa(hw, i, 0, zbuf, 0, 0, 0);
> +       }
> +
> +       /* reload the Tx keys */
> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
> +               struct tx_sa *tsa = &adapter->tx_sa_tbl[i];
> +
> +               if (tsa->used)
> +                       ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt);
> +               else
> +                       ixgbe_ipsec_set_tx_sa(hw, i, zbuf, 0);
> +       }
> +}
> +
> +/**
> + * ixgbe_ipsec_find_empty_idx - find the first unused security parameter index
> + * @adapter: pointer to adapter struct
> + * @rxtable: true if we need to look in the Rx table
> + *
> + * Returns the first unused index in either the Rx or Tx SA table
> + **/
> +static int ixgbe_ipsec_find_empty_idx(struct ixgbe_adapter *adapter,
> +                                     bool rxtable)
> +{
> +       u32 i;
> +
> +       if (rxtable) {
> +               if (adapter->num_rx_sa == IXGBE_IPSEC_MAX_SA_COUNT)
> +                       return -ENOSPC;
> +
> +               /* search rx sa table */
> +               for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
> +                       if (!adapter->rx_sa_tbl[i].used)
> +                               return i;
> +               }
> +       } else {
> +               if (adapter->num_rx_sa == IXGBE_IPSEC_MAX_SA_COUNT)
> +                       return -ENOSPC;
> +
> +               /* search tx sa table */
> +               for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
> +                       if (!adapter->tx_sa_tbl[i].used)
> +                               return i;
> +               }
> +       }
> +
> +       return -ENOSPC;
> +}
> +
> +/**
> + * ixgbe_ipsec_find_rx_state - find the state that matches
> + * @adapter: pointer to adapter struct
> + * @daddr: inbound address to match
> + * @proto: protocol to match
> + * @spi: SPI to match
> + *
> + * Returns a pointer to the matching SA state information
> + **/
> +static struct xfrm_state *ixgbe_ipsec_find_rx_state(
> +                                       struct ixgbe_adapter *adapter,
> +                                       __be32 daddr, u8 proto, __be32 spi)
> +{
> +       struct rx_sa *rsa;
> +       u32 i;
> +
> +       /* search rx sa table */
> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
> +               rsa = &adapter->rx_sa_tbl[i];
> +
> +               if (!rsa->used)
> +                       continue;
> +
> +               if (spi == rsa->xs->id.spi &&
> +                   daddr == rsa->xs->id.daddr.a4 &&
> +                   proto == rsa->xs->id.proto) {
> +                       xfrm_state_hold(adapter->rx_sa_tbl[i].xs);
> +                       return adapter->rx_sa_tbl[i].xs;
> +               }
> +       }
> +
> +       return NULL;

This is going to need to change if you hope to support any significant
number of security associations with any sort of support. You would
probably be much better served using some sort of hash instead of
trying to walk a linear list for every lookup.

> +}
> +
> +/**
> + * ixgbe_ipsec_parse_proto_keys - find the key and salt based on the protocol
> + * @xs: pointer to xfrm_state struct
> + * @mykey: pointer to key array to populate
> + * @mysalt: pointer to salt value to populate
> + *
> + * This copies the protocol keys and salt to our own data tables.  The
> + * 82599 family only supports the one algorithm.
> + **/
> +static int ixgbe_ipsec_parse_proto_keys(struct xfrm_state *xs,
> +                                       u32 *mykey, u32 *mysalt)
> +{
> +       struct net_device *dev = xs->xso.dev;
> +       unsigned char *key_data;
> +       char *alg_name = NULL;
> +       char *aes_gcm_name = "rfc4106(gcm(aes))";
> +       int key_len;
> +
> +       if (xs->aead) {
> +               key_data = &xs->aead->alg_key[0];
> +               key_len = xs->aead->alg_key_len;
> +               alg_name = xs->aead->alg_name;
> +       } else {
> +               netdev_err(dev, "Unsupported IPsec algorithm\n");
> +               return -EINVAL;
> +       }
> +
> +       if (strcmp(alg_name, aes_gcm_name)) {
> +               netdev_err(dev, "Unsupported IPsec algorithm - please use %s\n",
> +                          aes_gcm_name);
> +               return -EINVAL;
> +       }
> +
> +       /* 160 accounts for 16 byte key and 4 byte salt */
> +       if (key_len == 128) {
> +               netdev_info(dev, "IPsec hw offload parameters missing 32 bit salt value\n");
> +       } else if (key_len != 160) {
> +               netdev_err(dev, "IPsec hw offload only supports keys up to 128 bits with a 32 bit salt\n");
> +               return -EINVAL;
> +       }
> +
> +       /* The key bytes come down in a bigendian array of bytes, and
> +        * salt is always the last 4 bytes of the key array.
> +        * We don't need to do any byteswapping.
> +        */
> +       memcpy(mykey, key_data, 16);
> +       if (key_len == 160)
> +               *mysalt = ((u32 *)key_data)[4];
> +       else
> +               *mysalt = 0;
> +
> +       return 0;
> +}
> +
> +/**
> + * ixgbe_ipsec_add_sa - program device with a security association
> + * @xs: pointer to transformer state struct
> + **/
> +static int ixgbe_ipsec_add_sa(struct xfrm_state *xs)
> +{
> +       struct net_device *dev = xs->xso.dev;
> +       struct ixgbe_adapter *adapter = netdev_priv(dev);
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       int checked, match, first;
> +       u32 sa_idx;
> +       int ret;
> +       int i;
> +
> +       if (xs->id.proto != IPPROTO_ESP && xs->id.proto != IPPROTO_AH) {
> +               netdev_err(dev, "Unsupported protocol 0x%04x for ipsec offload\n",
> +                          xs->id.proto);
> +               return -EINVAL;
> +       }
> +
> +       if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
> +               struct rx_sa rsa;
> +
> +               if (xs->calg) {
> +                       netdev_err(dev, "Compression offload not supported\n");
> +                       return -EINVAL;
> +               }
> +
> +               /* find the first unused index */
> +               sa_idx = ixgbe_ipsec_find_empty_idx(adapter, true);
> +               if (sa_idx < 0) {
> +                       netdev_err(dev, "No space for SA in Rx table!\n");
> +                       return -ENOSPC;
> +               }
> +
> +               memset(&rsa, 0, sizeof(rsa));
> +               rsa.used = true;
> +               rsa.xs = xs;
> +
> +               if (rsa.xs->id.proto & IPPROTO_ESP)
> +                       rsa.decrypt = xs->ealg || xs->aead;
> +
> +               /* get the key and salt */
> +               ret = ixgbe_ipsec_parse_proto_keys(xs, rsa.key, &rsa.salt);
> +               if (ret) {
> +                       netdev_err(dev, "Failed to get key data for Rx SA table\n");
> +                       return ret;
> +               }
> +
> +               /* get ip for rx sa table */
> +               if (xs->xso.flags & XFRM_OFFLOAD_IPV6)
> +                       memcpy(rsa.ipaddr, &xs->id.daddr.a6, 16);
> +               else
> +                       memcpy(&rsa.ipaddr[3], &xs->id.daddr.a4, 4);
> +
> +               /* The HW does not have a 1:1 mapping from keys to IP addrs, so
> +                * check for a matching IP addr entry in the table.  If the addr
> +                * already exists, use it; else find an unused slot and add the
> +                * addr.  If one does not exist and there are no unused table
> +                * entries, fail the request.
> +                */
> +
> +               /* Find an existing match or first not used, and stop looking
> +                * after we've checked all we know we have.
> +                */
> +               checked = 0;
> +               match = -1;
> +               first = -1;
> +               for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT &&
> +                           (checked < adapter->num_rx_sa || first < 0); i++) {
> +                       if (adapter->rx_ip_sa_tbl[i].used) {
> +                               if (!memcmp(adapter->rx_ip_sa_tbl[i].ipaddr,
> +                                           rsa.ipaddr, sizeof(rsa.ipaddr))) {
> +                                       match = i;
> +                                       break;
> +                               }
> +                               checked++;
> +                       } else if (first < 0) {
> +                               first = i;  /* track the first empty seen */
> +                       }
> +               }
> +
> +               if (adapter->num_rx_sa == 0)
> +                       first = 0;
> +
> +               if (match >= 0) {
> +                       /* addrs are the same, we should use this one */
> +                       rsa.iptbl_ind = match;
> +                       adapter->rx_ip_sa_tbl[match].ref_cnt++;
> +
> +               } else if (first >= 0) {
> +                       /* no matches, but here's an empty slot */
> +                       rsa.iptbl_ind = first;
> +
> +                       memcpy(adapter->rx_ip_sa_tbl[first].ipaddr,
> +                              rsa.ipaddr, sizeof(rsa.ipaddr));
> +                       adapter->rx_ip_sa_tbl[first].ref_cnt = 1;
> +                       adapter->rx_ip_sa_tbl[first].used = true;
> +
> +                       ixgbe_ipsec_set_rx_ip(hw, rsa.iptbl_ind, rsa.ipaddr);
> +
> +               } else {
> +                       /* no match and no empty slot */
> +                       netdev_err(dev, "No space for SA in Rx IP SA table\n");
> +                       memset(&rsa, 0, sizeof(rsa));
> +                       return -ENOSPC;
> +               }
> +
> +               rsa.mode = IXGBE_RXMOD_VALID;
> +               if (rsa.xs->id.proto & IPPROTO_ESP)
> +                       rsa.mode |= IXGBE_RXMOD_PROTO_ESP;
> +               if (rsa.decrypt)
> +                       rsa.mode |= IXGBE_RXMOD_DECRYPT;
> +               if (rsa.xs->xso.flags & XFRM_OFFLOAD_IPV6)
> +                       rsa.mode |= IXGBE_RXMOD_IPV6;
> +
> +               /* the preparations worked, so save the info */
> +               memcpy(&adapter->rx_sa_tbl[sa_idx], &rsa, sizeof(rsa));
> +
> +               ixgbe_ipsec_set_rx_sa(hw, sa_idx, rsa.xs->id.spi, rsa.key,
> +                                     rsa.salt, rsa.mode, rsa.iptbl_ind);
> +               xs->xso.offload_handle = sa_idx + IXGBE_IPSEC_BASE_RX_INDEX;
> +
> +               adapter->num_rx_sa++;
> +       } else {
> +               struct tx_sa tsa;
> +
> +               /* find the first unused index */
> +               sa_idx = ixgbe_ipsec_find_empty_idx(adapter, false);
> +               if (sa_idx < 0) {
> +                       netdev_err(dev, "No space for SA in Tx table\n");
> +                       return -ENOSPC;
> +               }
> +
> +               memset(&tsa, 0, sizeof(tsa));
> +               tsa.used = true;
> +               tsa.xs = xs;
> +
> +               if (xs->id.proto & IPPROTO_ESP)
> +                       tsa.encrypt = xs->ealg || xs->aead;
> +
> +               ret = ixgbe_ipsec_parse_proto_keys(xs, tsa.key, &tsa.salt);
> +               if (ret) {
> +                       netdev_err(dev, "Failed to get key data for Tx SA table\n");
> +                       memset(&tsa, 0, sizeof(tsa));
> +                       return ret;
> +               }
> +
> +               /* the preparations worked, so save the info */
> +               memcpy(&adapter->tx_sa_tbl[sa_idx], &tsa, sizeof(tsa));
> +
> +               ixgbe_ipsec_set_tx_sa(hw, sa_idx, tsa.key, tsa.salt);
> +
> +               xs->xso.offload_handle = sa_idx + IXGBE_IPSEC_BASE_TX_INDEX;
> +
> +               adapter->num_tx_sa++;
> +       }
> +
> +       /* enable the engine if not already warmed up */
> +       if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)) {
> +               ixgbe_ipsec_start_engine(adapter);
> +               adapter->flags2 |= IXGBE_FLAG2_IPSEC_ENABLED;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * ixgbe_ipsec_del_sa - clear out this specific SA
> + * @xs: pointer to transformer state struct
> + **/
> +static void ixgbe_ipsec_del_sa(struct xfrm_state *xs)
> +{
> +       struct net_device *dev = xs->xso.dev;
> +       struct ixgbe_adapter *adapter = netdev_priv(dev);
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       u32 zerobuf[4] = {0, 0, 0, 0};
> +       u16 sa_idx;
> +
> +       if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
> +               struct rx_sa *rsa;
> +               u8 iptbl_ind;
> +
> +               sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_RX_INDEX;
> +               rsa = &adapter->rx_sa_tbl[sa_idx];
> +
> +               if (!rsa->used) {
> +                       netdev_err(dev, "Invalid Rx SA selected sa_idx=%d offload_handle=%lu\n",
> +                                  sa_idx, xs->xso.offload_handle);
> +                       return;
> +               }
> +
> +               /* if the IP table entry is referenced by only this SA,
> +                * i.e. ref_cnt is only 1, clear the IP table entry as well
> +                */
> +               iptbl_ind = rsa->iptbl_ind;
> +               if (adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt > 0) {
> +                       adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt--;
> +
> +                       if (!adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt) {
> +                               memset(&adapter->rx_ip_sa_tbl[iptbl_ind], 0,
> +                                      sizeof(struct rx_ip_sa));
> +                               ixgbe_ipsec_set_rx_ip(hw, iptbl_ind, zerobuf);
> +                       }
> +               }
> +
> +               /* clear the SA table entry */
> +               memset(rsa, 0, sizeof(struct rx_sa));
> +               ixgbe_ipsec_set_rx_sa(hw, sa_idx, 0, zerobuf, 0, 0, 0);
> +
> +               adapter->num_rx_sa--;
> +
> +       } else {
> +               sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
> +
> +               if (!adapter->tx_sa_tbl[sa_idx].used) {
> +                       netdev_err(dev, "Invalid Tx SA selected sa_idx=%d offload_handle=%lu\n",
> +                                  sa_idx, xs->xso.offload_handle);
> +                       return;
> +               }
> +
> +               /* clear the SA table entry */
> +               memset(&adapter->tx_sa_tbl[sa_idx], 0, sizeof(struct tx_sa));
> +               ixgbe_ipsec_set_tx_sa(hw, sa_idx, zerobuf, 0);
> +
> +               adapter->num_tx_sa--;
> +       }
> +
> +       /* if there are no SAs left, stop the engine to save energy */
> +       if (adapter->num_rx_sa == 0 && adapter->num_tx_sa == 0) {
> +               adapter->flags2 &= ~IXGBE_FLAG2_IPSEC_ENABLED;
> +               ixgbe_ipsec_stop_engine(adapter);
> +       }
> +}
> +
> +/**
> + * ixgbe_ipsec_free - called by xfrm garbage collections
> + * @xs: pointer to transformer state struct
> + *
> + * We don't have any garbage to collect, so we shouldn't even bother
> + * implementing this function.
> + **/
> +static void ixgbe_ipsec_free(struct xfrm_state *xs)
> +{
> +}
> +
> +/**
> + * ixgbe_ipsec_offload_ok - can this packet use the xfrm hw offload
> + * @skb: current data packet
> + * @xs: pointer to transformer state struct
> + **/
> +static bool ixgbe_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
> +{
> +       if (xs->props.family == AF_INET) {
> +               /* Offload with IPv4 options is not supported yet */
> +               if (ip_hdr(skb)->ihl > 5)
> +                       return false;
> +       } else {
> +               /* Offload with IPv6 extension headers is not support yet */
> +               if (ipv6_ext_hdr(ipv6_hdr(skb)->nexthdr))
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
> +static const struct xfrmdev_ops ixgbe_xfrmdev_ops = {
> +       .xdo_dev_state_add = ixgbe_ipsec_add_sa,
> +       .xdo_dev_state_delete = ixgbe_ipsec_del_sa,
> +       .xdo_dev_state_free = ixgbe_ipsec_free,
> +       .xdo_dev_offload_ok = ixgbe_ipsec_offload_ok,
> +};
> +
> +/**
> + * ixgbe_ipsec_tx - setup Tx flags for ipsec offload
> + * @tx_ring: outgoing context
> + * @skb: current data packet
> + * @itd: ipsec Tx data for later use in building context descriptor
> + **/
> +int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
> +                  struct ixgbe_ipsec_tx_data *itd)
> +{
> +       struct ixgbe_adapter *adapter = netdev_priv(tx_ring->netdev);
> +       struct xfrm_state *xs;
> +       struct tx_sa *tsa;
> +
> +       /* TODO: not supporting IPv6 yet */
> +       if (skb->protocol != htons(ETH_P_IP))
> +               return 0;
> +

This is a bit too simplistic, and ignores VLANs. You would probably be
much better off moving this code into ixgbe_main and keeping it
somewhere close to ixgbe_xmit_frame_ring. Also instead of passing the
skb, you might look at passing the first tx_buffer_info structure for
the xmit, it would already have ETH_P_IP, and it would have skipped
over any VLAN tags if they were present.

> +       if ((ip_hdr(skb)->protocol != IPPROTO_ESP) &&
> +           (ip_hdr(skb)->protocol != IPPROTO_AH))
> +               return 0;
> +
> +       if (!skb->sp) {
> +               netdev_err(tx_ring->netdev, "%s: no xfrm state skb->sp = %p encap=%d skb=%p\n",
> +                          __func__, skb->sp, skb->encapsulation, skb);
> +               return 0;
> +       }
> +

I would make this check the first check in this function. Also the
warning doesn't make any sense, is the assumption that if you support
IPsec offload you are going to offload all IPsec frames? Seems like
this is something where you could have a mix of both offloaded and
non-offloaded frames. In addition this could cause issues if a
interface is just performing routing between two interfaces that are
passing IPsec traffic.

> +       if (!skb->sp->len) {
> +               netdev_err(tx_ring->netdev, "%s: no xfrm state len = %d\n",
> +                          __func__, skb->sp->len);
> +               return 0;
> +       }
> +
> +       xs = xfrm_input_state(skb);
> +       if (!xs) {
> +               netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs = %p\n",
> +                          __func__, xs);
> +               return 0;
> +       }
> +
> +       itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
> +       if (itd->sa_idx < 0 || itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
> +               netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d\n",
> +                          __func__, itd->sa_idx);
> +               return 0;
> +       }

Does sa_idx need to be a signed value? If not you might do a unsigned
comparison of the value and avoid having to do two comparisons.

> +
> +       tsa = &adapter->tx_sa_tbl[itd->sa_idx];
> +       if (!tsa->used) {
> +               netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
> +                          __func__, itd->sa_idx);
> +               return 0;
> +       }
> +
> +       itd->flags = 0;
> +       if (xs->id.proto == IPPROTO_ESP) {
> +               itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP |
> +                             IXGBE_ADVTXD_TUCMD_L4T_TCP;

So you should probably drop the TCP flag here. It might explain some
of the issues you are having with TSO and checksum offloads being
enabled. That bit should only be set if we are requesting a TCP
checksum offload.

> +               if (skb->protocol == htons(ETH_P_IP))
> +                       itd->flags |= IXGBE_ADVTXD_TUCMD_IPV4;

Same thing for this bit. It assumes you want an IPv4 checksum offload.
Normally it is only set for TSO packets where we have to update the IP
header checksum.

> +               itd->trailer_len = xs->props.trailer_len;
> +       }

You might look at just combining the flags and trailer_len field into
one field and be done with it. Then all you have to do is pre-shift
the fields before you write them into the appropriate spots.

> +       if (tsa->encrypt)
> +               itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN;
> +
> +       return 1;
> +}
> +
> +/**
> + * ixgbe_ipsec_process_rx_offload - decode ipsec bits from Rx descriptor
> + * @rx_ring: receiving ring
> + * @rx_desc: receive data descriptor
> + * @skb: current data packet
> + *
> + * Determine if there was an ipsec encapsulation noticed, and if so set up
> + * the resulting status for later in the receive stack.
> + **/
> +void ixgbe_ipsec_process_rx_offload(struct ixgbe_ring *rx_ring,
> +                                   union ixgbe_adv_rx_desc *rx_desc,
> +                                   struct sk_buff *skb)
> +{
> +       struct ixgbe_adapter *adapter = netdev_priv(rx_ring->netdev);
> +       u16 pkt_info = le16_to_cpu(rx_desc->wb.lower.lo_dword.hs_rss.pkt_info);
> +       u32 status_error = le32_to_cpu(rx_desc->wb.upper.status_error);
> +       u16 ipsec_pkt_types = IXGBE_RXDADV_PKTTYPE_IPSEC_AH |
> +                               IXGBE_RXDADV_PKTTYPE_IPSEC_ESP;
> +       struct xfrm_offload *xo = NULL;
> +       struct xfrm_state *xs = NULL;
> +       struct iphdr *iph;
> +       u32 crypto_status = 0;
> +       u8 *c_hdr;
> +       __be32 spi;
> +       u8 proto;
> +

Probably the first check here should be for the SECP bit in the
descriptor meaning that there was a security encapsulation recognized.
Without that check you could end up wasting a ton of cycles on IPsec
packets that aren't even being offloaded.

You might even be able to get away with checking for the SECP bit
being set instead of having to check the adapter->flag2 value that you
do in later patches to determine if you are going to handle the ipsec
offload in the Rx path.

> +       /* TODO: we're only handling ipv4 at the moment */
> +       if (!(pkt_info & IXGBE_RXDADV_PKTTYPE_IPV4))
> +               return;
> +
> +       /* look for our packet types */
> +       if (!(pkt_info & ipsec_pkt_types))
> +               return;
> +

I suspect you only need this check, you probably don't need the IPv4
check. You would need to test to verify, but if the hardware doesn't
support IPv6 offload with these protocols there is a good chance the
parser won't handle it either.

In addition you might want to go through and verify the upper limit as
well.  Basically if you verified that at least one bit is set, you
should probably verify that only one bit is set as well. Then you
could turn your switch statement below into just an if statement with
two cases.

> +       /* don't process this if no offload work was done */
> +       if (!(status_error & IXGBE_RXDADV_STAT_SECP))
> +               return;
> +
> +       iph = (struct iphdr *)(skb_mac_header(skb) + ETH_HLEN);
> +       c_hdr = (u8 *)iph + iph->ihl * 4;

This code is assuming quite a bit as it assumes no VLANs are present
and I think they could be. I believe you are currently handling this
after eth_type_trans is already called, otherwise skb_mac_header
wouldn't work since it wouldn't be populated. It might be better to do
all this before eth_type_trans is called and instead just use
skb->data. You may want to look at using something like
__vlan_get_protocol or code similar.

> +       switch (pkt_info & ipsec_pkt_types) {
> +       case IXGBE_RXDADV_PKTTYPE_IPSEC_AH:
> +               spi = ((struct ip_auth_hdr *)c_hdr)->spi;
> +               proto = IPPROTO_AH;
> +               break;
> +       case IXGBE_RXDADV_PKTTYPE_IPSEC_ESP:
> +               spi = ((struct ip_esp_hdr *)c_hdr)->spi;
> +               proto = IPPROTO_ESP;
> +               break;
> +       default:
> +               spi = 0;
> +               proto = 0;
> +               break;
> +       }
> +
> +       xs = ixgbe_ipsec_find_rx_state(adapter, iph->daddr, proto, spi);
> +       if (unlikely(!xs))
> +               return;

Once again this function will need to be updated since it is in the Rx
hotpath it would be best to look at possibly having it generate a hash
and then use a hash table instead of an array for the lookup.

> +
> +       /* decode error bits, if any */
> +       if ((status_error & IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) == IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) {
> +               if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_AH)
> +                       crypto_status = CRYPTO_TRANSPORT_AH_AUTH_FAILED;
> +               else if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_ESP)
> +                       crypto_status = CRYPTO_TRANSPORT_ESP_AUTH_FAILED;
> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL) {
> +               crypto_status = CRYPTO_INVALID_PROTOCOL;
> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH) {
> +               crypto_status = CRYPTO_INVALID_PACKET_SYNTAX;
> +       } else {
> +               crypto_status = CRYPTO_SUCCESS;
> +       }
> +

You might look at converting this into a look-up table since you have
essentially 2 protocols (AH or ESP), and 4 possible return values that
have to be translated int the final result value. A single 2
dimensional 8 byte array should be enough to handle all the possible
crypto_status return values.

> +       skb->sp = secpath_dup(skb->sp);
> +       if (unlikely(!skb->sp))
> +               return;
> +

It seems like this should be done as soon as you have found your
transform state. Otherwise there isn't much point in decoding the
status_error bits.

> +       skb->sp->xvec[skb->sp->len++] = xs;
> +       skb->sp->olen++;
> +       xo = xfrm_offload(skb);
> +       xo->flags = CRYPTO_DONE;
> +       xo->status = crypto_status;
> +
> +       if (xo->status == CRYPTO_SUCCESS)
> +               rx_ring->rx_stats.ipsec_offloads++;
> +       else
> +               rx_ring->rx_stats.ipsec_offload_faileds++;

You might rename offload_faileds to offload_errors, since the word
"failed" isn't really something to pluralize.

> +}
> +
> +/**
> + * ixgbe_init_ipsec_offload - initialize security registers for IPSec operation
> + * @adapter: board private structure
> + **/
> +void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter)
> +{
> +       // TODO: enable and support checksum offload with ESP
> +       //netdev_features_t esp_feats = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM;
> +       netdev_features_t esp_feats = NETIF_F_HW_ESP;
> +       size_t size;
> +
> +       /* allocate SA tables for rx, tx, and ip table */
> +       size = sizeof(struct rx_sa) * IXGBE_IPSEC_MAX_SA_COUNT;
> +       adapter->rx_sa_tbl = kzalloc(size, GFP_KERNEL);
> +       if (!adapter->rx_sa_tbl)
> +               goto err;
> +
> +       size = sizeof(struct tx_sa) * IXGBE_IPSEC_MAX_SA_COUNT;
> +       adapter->tx_sa_tbl = kzalloc(size, GFP_KERNEL);
> +       if (!adapter->tx_sa_tbl)
> +               goto err;
> +
> +       size = sizeof(struct rx_ip_sa) * IXGBE_IPSEC_MAX_RX_IP_COUNT;
> +       adapter->rx_ip_sa_tbl = kzalloc(size, GFP_KERNEL);
> +       if (!adapter->rx_ip_sa_tbl)
> +               goto err;
> +
> +       adapter->num_rx_sa = 0;
> +       adapter->num_tx_sa = 0;
> +       ixgbe_ipsec_clear_hw_tables(adapter);
> +       ixgbe_ipsec_stop_engine(adapter);
> +
> +       adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops;
> +       adapter->netdev->features |= esp_feats;
> +       adapter->netdev->hw_enc_features |= esp_feats;
> +
> +       return;
> +
> +err:
> +       kfree(adapter->rx_ip_sa_tbl);
> +       kfree(adapter->rx_sa_tbl);
> +       kfree(adapter->tx_sa_tbl);
> +       netdev_err(adapter->netdev, "Unable to allocate memory for SA tables");
> +}
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
> new file mode 100644
> index 0000000..265950d
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
> @@ -0,0 +1,80 @@
> +/*******************************************************************************
> +
> +  Intel 10 Gigabit PCI Express Linux driver
> +  Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
> +
> +  This program is free software; you can redistribute it and/or modify it
> +  under the terms and conditions of the GNU General Public License,
> +  version 2, as published by the Free Software Foundation.
> +
> +  This program is distributed in the hope it will be useful, but WITHOUT
> +  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> +  more details.
> +
> +  You should have received a copy of the GNU General Public License along with
> +  this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +  The full GNU General Public License is included in this distribution in
> +  the file called "COPYING".
> +
> +  Contact Information:
> +  Linux NICS <linux.nics@intel.com>
> +  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
> +  Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
> +
> +*******************************************************************************/
> +
> +#ifndef _IXGBE_IPSEC_H_
> +#define _IXGBE_IPSEC_H_
> +
> +#define IXGBE_IPSEC_MAX_SA_COUNT       1024
> +#define IXGBE_IPSEC_MAX_RX_IP_COUNT    128
> +#define IXGBE_IPSEC_BASE_RX_INDEX      IXGBE_IPSEC_MAX_SA_COUNT
> +#define IXGBE_IPSEC_BASE_TX_INDEX      (IXGBE_IPSEC_MAX_SA_COUNT * 2)
> +
> +#define IXGBE_RXTXIDX_IPS_EN           0x00000001
> +#define IXGBE_RXIDX_TBL_MASK           0x00000006
> +#define IXGBE_RXIDX_TBL_IP             0x00000002
> +#define IXGBE_RXIDX_TBL_SPI            0x00000004
> +#define IXGBE_RXIDX_TBL_KEY            0x00000006
> +#define IXGBE_RXTXIDX_IDX_MASK         0x00001ff8
> +#define IXGBE_RXTXIDX_IDX_READ         0x40000000
> +#define IXGBE_RXTXIDX_IDX_WRITE                0x80000000
> +
> +#define IXGBE_RXMOD_VALID              0x00000001
> +#define IXGBE_RXMOD_PROTO_ESP          0x00000004
> +#define IXGBE_RXMOD_DECRYPT            0x00000008
> +#define IXGBE_RXMOD_IPV6               0x00000010
> +
> +struct rx_sa {
> +       struct xfrm_state *xs;
> +       u32 ipaddr[4];
> +       u32 key[4];
> +       u32 salt;
> +       u32 mode;
> +       u8  iptbl_ind;
> +       bool used;
> +       bool decrypt;
> +};
> +
> +struct rx_ip_sa {
> +       u32 ipaddr[4];
> +       u32 ref_cnt;
> +       bool used;
> +};
> +
> +struct tx_sa {
> +       struct xfrm_state *xs;
> +       u32 key[4];
> +       u32 salt;
> +       bool encrypt;
> +       bool used;
> +};
> +
> +struct ixgbe_ipsec_tx_data {
> +       u32 flags;
> +       u16 trailer_len;
> +       u16 sa_idx;
> +};
> +#endif /* _IXGBE_IPSEC_H_ */
> --
> 2.7.4
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 3/9] ixgbe: add the ipsec support code files

[Shannon Nelson]

Again, thanks for the review on this early code.  I have a few responses 
below.

sln

On 11/17/2017 1:30 PM, Alexander Duyck wrote:
> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> This adds the ipsec support code into the driver, but doesn't yet
>> include it into the compile.
>>
>> It is possible that the .h file contents could simply go into
>> the ixgbe_type.h and ixgbe.h files.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> 
> Generally I really hate it when code is just thrown in like this as it
> is really difficult to review and/or debug. I would really much prefer
> to have this code added as it is needed for specific functionality.
> 
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 1133 ++++++++++++++++++++++++
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h |   80 ++
>>   2 files changed, 1213 insertions(+)
>>   create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>   create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>> new file mode 100644
>> index 0000000..177b915
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>> @@ -0,0 +1,1133 @@
>> +/*******************************************************************************
>> + *
>> + * Intel 10 Gigabit PCI Express Linux driver
>> + * Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + *
>> + * The full GNU General Public License is included in this distribution in
>> + * the file called "COPYING".
>> + *
>> + * Contact Information:
>> + * Linux NICS <linux.nics@intel.com>
>> + * e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
>> + * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
>> + *
>> + ******************************************************************************/
>> +
>> +#include <linux/types.h>
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/vmalloc.h>
>> +#include <linux/string.h>
>> +#include <linux/in.h>
>> +#include <linux/ip.h>
>> +#include <linux/tcp.h>
>> +#include <linux/ipv6.h>
>> +#include <net/xfrm.h>
>> +#include <crypto/aead.h>
>> +
>> +#include "ixgbe.h"
>> +#include "ixgbe_type.h"
>> +#include "ixgbe_ipsec.h"
>> +
>> +/**
>> + * ixgbe_ipsec_dump_info - dump in-memory SA tables for debugging
>> + * @adapter: pointer to adapter structure
>> + **/
>> +void ixgbe_ipsec_dump_info(struct ixgbe_adapter *adapter)
>> +{
>> +       struct net_device *dev = adapter->netdev;
>> +       int i;
>> +
>> +       netdev_info(dev, "IXGBE_IPSEC_INFO_DUMP\n");
>> +
>> +       netdev_info(dev, "  Tx SA table\n");
>> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
>> +               struct tx_sa *tsa = &adapter->tx_sa_tbl[i];
>> +
>> +               if (!tsa->used)
>> +                       continue;
>> +
>> +               netdev_info(dev, " tx_sa_tbl[%i] key = 0x%08x %08x %08x %08x\n",
>> +                           i, tsa->key[0], tsa->key[1],
>> +                           tsa->key[2], tsa->key[3]);
>> +               netdev_info(dev, "              salt = 0x%08x encrypt = %d\n",
>> +                           tsa->salt, tsa->encrypt);
>> +       }
>> +
>> +       netdev_info(dev, "  Rx SA table\n");
>> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
>> +               struct rx_sa *rsa = &adapter->rx_sa_tbl[i];
>> +
>> +               if (!rsa->used)
>> +                       continue;
>> +
>> +               netdev_info(dev, "rx_sa_tbl[%i] ipaddr = 0x%08x %08x %08x %08x\n",
>> +                           i, rsa->ipaddr[0], rsa->ipaddr[1],
>> +                           rsa->ipaddr[2], rsa->ipaddr[3]);
>> +               netdev_info(dev, "             spi = 0x%08x proto = 0x%x iptbl_ind = 0x%08x mode = 0x%04x\n",
>> +                           rsa->xs->id.spi, rsa->xs->id.proto,
>> +                           rsa->iptbl_ind, rsa->mode);
>> +               netdev_info(dev, "             key = 0x%08x %08x %08x %08x\n",
>> +                           rsa->key[0], rsa->key[1],
>> +                           rsa->key[2], rsa->key[3]);
>> +               netdev_info(dev, "             salt = 0x%08x decrypt = %d xs = %p\n",
>> +                           rsa->salt, rsa->decrypt, rsa->xs);
>> +       }
>> +
>> +       netdev_info(dev, "  Rx IP table\n");
>> +       for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
>> +               struct rx_ip_sa *ipsa = &adapter->rx_ip_sa_tbl[i];
>> +
>> +               if (!ipsa->used)
>> +                       continue;
>> +
>> +               netdev_info(dev, "rx_ip_tbl[%i] ipaddr = 0x%08x %08x %08x %08x ref_cnt = 0x%08x\n",
>> +                           i, ipsa->ipaddr[0], ipsa->ipaddr[1],
>> +                           ipsa->ipaddr[2], ipsa->ipaddr[3],
>> +                           ipsa->ref_cnt);
>> +       }
>> +
>> +       netdev_info(dev, "  Offload counts\n");
>> +       for (i = 0; i < adapter->num_rx_queues; i++) {
>> +               struct ixgbe_ring *rx_ring = adapter->rx_ring[i];
>> +
>> +               if (rx_ring->rx_stats.ipsec_offloads ||
>> +                   rx_ring->rx_stats.ipsec_offload_faileds) {
>> +                       netdev_info(dev, "rx_ring[%i] ipsec_offloads = %llu ipsec_offload_faileds = %llu\n",
>> +                                   i, rx_ring->rx_stats.ipsec_offloads,
>> +                                   rx_ring->rx_stats.ipsec_offload_faileds);
>> +               }
>> +       }
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_dump_hw - dump the contents of the hw tables
>> + * @adapter: pointer to adapter structure
>> + **/
>> +void ixgbe_ipsec_dump_hw(struct ixgbe_adapter *adapter)
>> +{
>> +       struct net_device *dev = adapter->netdev;
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       int t_idx, i;
>> +       u32 reg;
>> +
>> +       netdev_info(dev, "IPsec HW\n");
>> +
>> +       netdev_info(dev, "SECRXCTRL=0x%08x  SECRXSTAT=0x%08x\n",
>> +                   IXGBE_READ_REG(hw, IXGBE_SECRXCTRL),
>> +                   IXGBE_READ_REG(hw, IXGBE_SECRXSTAT));
>> +
>> +       netdev_info(dev, "SECTXCTRL=0x%08x  SECTXSTAT=0x%08x, SECTXBUFFAF=0x%08x, SECTXMINIFG=0x%08x\n",
>> +                   IXGBE_READ_REG(hw, IXGBE_SECTXCTRL),
>> +                   IXGBE_READ_REG(hw, IXGBE_SECTXSTAT),
>> +                   IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF),
>> +                   IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG));
>> +
>> +       netdev_info(dev, "RxIdx=0x%08x  TxIdx=0x%08x\n",
>> +                   IXGBE_READ_REG(hw, IXGBE_IPSRXIDX),
>> +                   IXGBE_READ_REG(hw, IXGBE_IPSTXIDX));
>> +
>> +       /* Tx SA Table */
>> +       netdev_info(dev, "Tx SA HW Table\n");
>> +       for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_SA_COUNT; t_idx++) {
>> +               u32 key[4];
>> +               u32 salt;
>> +               u32 anybit = 0;
>> +
>> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
>> +               reg &= IXGBE_RXTXIDX_IPS_EN;
>> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ;
>> +               IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, reg);
>> +               for (i = 0; i < 4; i++) {
>> +                       key[i] = IXGBE_READ_REG(hw, IXGBE_IPSTXKEY(i));
>> +                       anybit |= key[i];
>> +               }
>> +               salt = IXGBE_READ_REG(hw, IXGBE_IPSTXSALT);
>> +               anybit |= salt;
>> +
>> +               if (anybit)
>> +                       netdev_info(dev, "  SA[%d] key=0x%08x 0x%08x 0x%08x 0x%08x salt=0x%08x\n",
>> +                                   t_idx, key[3], key[2], key[1], key[0],
>> +                                   salt);
>> +       }
>> +
>> +       /* Rx SA Table */
>> +       netdev_info(dev, "Rx SA HW Table\n");
>> +       for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_SA_COUNT; t_idx++) {
>> +               u32 key[4];
>> +               u32 ip_idx;
>> +               __be32 spi;
>> +               u32 salt;
>> +               u32 mode;
>> +               u32 anybit = 0;
>> +
>> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
>> +               reg &= IXGBE_RXTXIDX_IPS_EN;
>> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_SPI;
>> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
>> +               spi = IXGBE_READ_REG(hw, IXGBE_IPSRXSPI);
>> +               ip_idx = IXGBE_READ_REG(hw, IXGBE_IPSRXIPIDX);
>> +
>> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
>> +               reg &= IXGBE_RXTXIDX_IPS_EN;
>> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_KEY;
>> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
>> +               for (i = 0; i < 4; i++) {
>> +                       key[i] = IXGBE_READ_REG(hw, IXGBE_IPSRXKEY(i));
>> +                       anybit |= key[i];
>> +               }
>> +               salt = IXGBE_READ_REG(hw, IXGBE_IPSRXSALT);
>> +               mode = IXGBE_READ_REG(hw, IXGBE_IPSRXMOD);
>> +
>> +               anybit |= salt | mode | spi | ip_idx;
>> +               if (anybit)
>> +                       netdev_info(dev, "  SA[%d] spi=0x%08x key=0x%08x 0x%08x 0x%08x 0x%08x salt=0x%08x ipi=0x%08x mode=0x%08x\n",
>> +                                   t_idx, spi, key[3], key[2], key[1], key[0],
>> +                                   salt, ip_idx, mode);
>> +       }
>> +
>> +       /* Rx IP Table */
>> +       netdev_info(dev, "Rx IP HW Table\n");
>> +       for (t_idx = 0; t_idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; t_idx++) {
>> +               u32 ipaddr[4];
>> +               u32 anybit = 0;
>> +
>> +               reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
>> +               reg &= IXGBE_RXTXIDX_IPS_EN;
>> +               reg |= t_idx << 3 | IXGBE_RXTXIDX_IDX_READ | IXGBE_RXIDX_TBL_IP;
>> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
>> +               for (i = 0; i < 4; i++) {
>> +                       ipaddr[i] = IXGBE_READ_REG(hw, IXGBE_IPSRXIPADDR(i));
>> +                       anybit |= ipaddr[i];
>> +               }
>> +
>> +               if (anybit)
>> +                       netdev_info(dev, "  IP[%d] 0x%08x 0x%08x 0x%08x 0x%08x\n",
>> +                                   t_idx, ipaddr[0], ipaddr[1],
>> +                                   ipaddr[2], ipaddr[3]);
>> +       }
>> +}
>> +
> 
> So all of this dumping code can go. I am pretty sure it exposes a
> pretty big security issue since it makes it far to easy for someone to
> get access to the keys and such for a given ipsec connection.

Yep - it all goes away with the debugfs code.

> 
>> +/**
>> + * ixgbe_ipsec_set_tx_sa - set the Tx SA registers
>> + * @hw: hw specific details
>> + * @idx: register index to write
>> + * @key: key byte array
>> + * @salt: salt bytes
>> + **/
>> +static void ixgbe_ipsec_set_tx_sa(struct ixgbe_hw *hw, u16 idx,
>> +                                 u32 key[], u32 salt)
>> +{
>> +       u32 reg;
>> +       int i;
>> +
>> +       for (i = 0; i < 4; i++)
>> +               IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(i), cpu_to_be32(key[3-i]));
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, cpu_to_be32(salt));
>> +       IXGBE_WRITE_FLUSH(hw);
>> +
>> +       reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
>> +       reg &= IXGBE_RXTXIDX_IPS_EN;
>> +       reg |= idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, reg);
>> +       IXGBE_WRITE_FLUSH(hw);
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_set_rx_item - set an Rx table item
>> + * @hw: hw specific details
>> + * @idx: register index to write
>> + * @tbl: table selector
>> + *
>> + * Trigger the device to store into a particular Rx table the
>> + * data that has already been loaded into the input register
>> + **/
>> +static void ixgbe_ipsec_set_rx_item(struct ixgbe_hw *hw, u16 idx, u32 tbl)
>> +{
>> +       u32 reg;
>> +
>> +       reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
>> +       reg &= IXGBE_RXTXIDX_IPS_EN;
>> +       reg |= tbl | idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
>> +       IXGBE_WRITE_FLUSH(hw);
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_set_rx_sa - set up the register bits to save SA info
>> + * @hw: hw specific details
>> + * @idx: register index to write
>> + * @spi: security parameter index
>> + * @key: key byte array
>> + * @salt: salt bytes
>> + * @mode: rx decrypt control bits
>> + * @ip_idx: index into IP table for related IP address
>> + **/
>> +static void ixgbe_ipsec_set_rx_sa(struct ixgbe_hw *hw, u16 idx, __be32 spi,
>> +                                 u32 key[], u32 salt, u32 mode, u32 ip_idx)
>> +{
>> +       int i;
>> +
>> +       /* store the SPI (in bigendian) and IPidx */
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, spi);
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, ip_idx);
>> +       IXGBE_WRITE_FLUSH(hw);
>> +
>> +       ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_SPI);
>> +
>> +       /* store the key, salt, and mode */
>> +       for (i = 0; i < 4; i++)
>> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(i), cpu_to_be32(key[3-i]));
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, cpu_to_be32(salt));
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, mode);
>> +       IXGBE_WRITE_FLUSH(hw);
>> +
>> +       ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_KEY);
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_set_rx_ip - set up the register bits to save SA IP addr info
>> + * @hw: hw specific details
>> + * @idx: register index to write
>> + * @addr: IP address byte array
>> + **/
>> +static void ixgbe_ipsec_set_rx_ip(struct ixgbe_hw *hw, u16 idx, u32 addr[])
>> +{
>> +       int i;
>> +
>> +       /* store the ip address */
>> +       for (i = 0; i < 4; i++)
>> +               IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(i), addr[i]);
>> +       IXGBE_WRITE_FLUSH(hw);
>> +
>> +       ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_IP);
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_clear_hw_tables - because some tables don't get cleared on reset
>> + * @hw: hw specific details
>> + **/
>> +void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
>> +{
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       u32 buf[4] = {0, 0, 0, 0};
>> +       u16 idx;
>> +
>> +       /* disable Rx and Tx SA lookup */
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
>> +
>> +       /* scrub the tables */
>> +       for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
>> +               ixgbe_ipsec_set_tx_sa(hw, idx, buf, 0);
>> +       adapter->num_tx_sa = 0;
>> +
>> +       for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
>> +               ixgbe_ipsec_set_rx_sa(hw, idx, 0, buf, 0, 0, 0);
>> +       adapter->num_rx_sa = 0;
>> +
>> +       for (idx = 0; idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; idx++)
>> +               ixgbe_ipsec_set_rx_ip(hw, idx, buf);
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_stop_data
>> + * @hw: hw specific details
>> + **/
>> +static void ixgbe_ipsec_stop_data(struct ixgbe_adapter *adapter)
>> +{
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       bool link = adapter->link_up;
>> +       u32 t_rdy, r_rdy;
>> +       u32 reg;
>> +
>> +       /* halt data paths */
>> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
>> +       reg |= IXGBE_SECTXCTRL_TX_DIS;
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
>> +
>> +       reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
>> +       reg |= IXGBE_SECRXCTRL_RX_DIS;
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
>> +
>> +       IXGBE_WRITE_FLUSH(hw);
>> +
>> +       /* If the tx fifo doesn't have link, but still has data,
>> +        * we can't clear the tx sec block.  Set the MAC loopback
>> +        * before block clear
>> +        */
>> +       if (!link) {
>> +               reg = IXGBE_READ_REG(hw, IXGBE_MACC);
>> +               reg |= IXGBE_MACC_FLU;
>> +               IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
>> +
>> +               reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
>> +               reg |= IXGBE_HLREG0_LPBK;
>> +               IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
>> +
>> +               IXGBE_WRITE_FLUSH(hw);
>> +               mdelay(3);
>> +       }
>> +
>> +       /* wait for the paths to empty */
>> +       do {
>> +               mdelay(10);
>> +               t_rdy = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT) &
>> +                       IXGBE_SECTXSTAT_SECTX_RDY;
>> +               r_rdy = IXGBE_READ_REG(hw, IXGBE_SECRXSTAT) &
>> +                       IXGBE_SECRXSTAT_SECRX_RDY;
>> +       } while (!t_rdy && !r_rdy);
>> +
>> +       /* undo loopback if we played with it earlier */
>> +       if (!link) {
>> +               reg = IXGBE_READ_REG(hw, IXGBE_MACC);
>> +               reg &= ~IXGBE_MACC_FLU;
>> +               IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
>> +
>> +               reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
>> +               reg &= ~IXGBE_HLREG0_LPBK;
>> +               IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
>> +
>> +               IXGBE_WRITE_FLUSH(hw);
>> +       }
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_stop_engine
>> + * @hw: hw specific details
>> + **/
>> +static void ixgbe_ipsec_stop_engine(struct ixgbe_adapter *adapter)
>> +{
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       u32 reg;
>> +
>> +       ixgbe_ipsec_stop_data(adapter);
>> +
>> +       /* disable Rx and Tx SA lookup */
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
>> +
>> +       /* disable the Rx and Tx engines and full packet store-n-forward */
>> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
>> +       reg |= IXGBE_SECTXCTRL_SECTX_DIS;
>> +       reg &= ~IXGBE_SECTXCTRL_STORE_FORWARD;
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
>> +
>> +       reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
>> +       reg |= IXGBE_SECRXCTRL_SECRX_DIS;
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
>> +
>> +       /* restore the "tx security buffer almost full threshold" to 0x250 */
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, 0x250);
>> +
>> +       /* Set minimum IFG between packets back to the default 0x1 */
>> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
>> +       reg = (reg & 0xfffffff0) | 0x1;
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
>> +
>> +       /* final set for normal (no ipsec offload) processing */
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_SECTX_DIS);
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, IXGBE_SECRXCTRL_SECRX_DIS);
>> +
>> +       IXGBE_WRITE_FLUSH(hw);
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_start_engine
>> + * @hw: hw specific details
>> + *
>> + * NOTE: this increases power consumption whether being used or not
>> + **/
>> +static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter)
>> +{
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       u32 reg;
>> +
>> +       ixgbe_ipsec_stop_data(adapter);
>> +
>> +       /* Set minimum IFG between packets to 3 */
>> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
>> +       reg = (reg & 0xfffffff0) | 0x3;
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
>> +
>> +       /* Set "tx security buffer almost full threshold" to 0x15 so that the
>> +        * almost full indication is generated only after buffer contains at
>> +        * least an entire jumbo packet.
>> +        */
>> +       reg = IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF);
>> +       reg = (reg & 0xfffffc00) | 0x15;
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, reg);
>> +
>> +       /* restart the data paths by clearing the DISABLE bits */
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, 0);
>> +       IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_STORE_FORWARD);
>> +
>> +       /* enable Rx and Tx SA lookup */
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, IXGBE_RXTXIDX_IPS_EN);
>> +       IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, IXGBE_RXTXIDX_IPS_EN);
>> +
>> +       IXGBE_WRITE_FLUSH(hw);
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset
>> + * @hw: hw specific details
>> + **/
>> +void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter)
>> +{
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       u32 zbuf[4] = {0, 0, 0, 0};
>> +       int i;
>> +
>> +       if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED))
>> +               return;
>> +
>> +       /* clean up the engine settings */
>> +       ixgbe_ipsec_stop_engine(adapter);
>> +
>> +       /* start the engine */
>> +       ixgbe_ipsec_start_engine(adapter);
>> +
>> +       /* reload the IP addrs */
>> +       for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
>> +               struct rx_ip_sa *ipsa = &adapter->rx_ip_sa_tbl[i];
>> +
>> +               if (ipsa->used)
>> +                       ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr);
>> +               else
>> +                       ixgbe_ipsec_set_rx_ip(hw, i, zbuf);
>> +       }
>> +
>> +       /* reload the Rx keys */
>> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
>> +               struct rx_sa *rsa = &adapter->rx_sa_tbl[i];
>> +
>> +               if (rsa->used)
>> +                       ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi,
>> +                                             rsa->key, rsa->salt,
>> +                                             rsa->mode, rsa->iptbl_ind);
>> +               else
>> +                       ixgbe_ipsec_set_rx_sa(hw, i, 0, zbuf, 0, 0, 0);
>> +       }
>> +
>> +       /* reload the Tx keys */
>> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
>> +               struct tx_sa *tsa = &adapter->tx_sa_tbl[i];
>> +
>> +               if (tsa->used)
>> +                       ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt);
>> +               else
>> +                       ixgbe_ipsec_set_tx_sa(hw, i, zbuf, 0);
>> +       }
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_find_empty_idx - find the first unused security parameter index
>> + * @adapter: pointer to adapter struct
>> + * @rxtable: true if we need to look in the Rx table
>> + *
>> + * Returns the first unused index in either the Rx or Tx SA table
>> + **/
>> +static int ixgbe_ipsec_find_empty_idx(struct ixgbe_adapter *adapter,
>> +                                     bool rxtable)
>> +{
>> +       u32 i;
>> +
>> +       if (rxtable) {
>> +               if (adapter->num_rx_sa == IXGBE_IPSEC_MAX_SA_COUNT)
>> +                       return -ENOSPC;
>> +
>> +               /* search rx sa table */
>> +               for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
>> +                       if (!adapter->rx_sa_tbl[i].used)
>> +                               return i;
>> +               }
>> +       } else {
>> +               if (adapter->num_rx_sa == IXGBE_IPSEC_MAX_SA_COUNT)
>> +                       return -ENOSPC;
>> +
>> +               /* search tx sa table */
>> +               for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
>> +                       if (!adapter->tx_sa_tbl[i].used)
>> +                               return i;
>> +               }
>> +       }
>> +
>> +       return -ENOSPC;
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_find_rx_state - find the state that matches
>> + * @adapter: pointer to adapter struct
>> + * @daddr: inbound address to match
>> + * @proto: protocol to match
>> + * @spi: SPI to match
>> + *
>> + * Returns a pointer to the matching SA state information
>> + **/
>> +static struct xfrm_state *ixgbe_ipsec_find_rx_state(
>> +                                       struct ixgbe_adapter *adapter,
>> +                                       __be32 daddr, u8 proto, __be32 spi)
>> +{
>> +       struct rx_sa *rsa;
>> +       u32 i;
>> +
>> +       /* search rx sa table */
>> +       for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
>> +               rsa = &adapter->rx_sa_tbl[i];
>> +
>> +               if (!rsa->used)
>> +                       continue;
>> +
>> +               if (spi == rsa->xs->id.spi &&
>> +                   daddr == rsa->xs->id.daddr.a4 &&
>> +                   proto == rsa->xs->id.proto) {
>> +                       xfrm_state_hold(adapter->rx_sa_tbl[i].xs);
>> +                       return adapter->rx_sa_tbl[i].xs;
>> +               }
>> +       }
>> +
>> +       return NULL;
> 
> This is going to need to change if you hope to support any significant
> number of security associations with any sort of support. You would
> probably be much better served using some sort of hash instead of
> trying to walk a linear list for every lookup.

This was simple enough to throw together and get out of the way fro 
initial debug on the rest of the code.  The Mellanox driver does a 
hashed list, which I may replicate here.  Alternatively, I'm also 
looking at using the xfrm_state_lookup() which may do what I need, since 
where this is getting used I don't need any driver specific information.

> 
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_parse_proto_keys - find the key and salt based on the protocol
>> + * @xs: pointer to xfrm_state struct
>> + * @mykey: pointer to key array to populate
>> + * @mysalt: pointer to salt value to populate
>> + *
>> + * This copies the protocol keys and salt to our own data tables.  The
>> + * 82599 family only supports the one algorithm.
>> + **/
>> +static int ixgbe_ipsec_parse_proto_keys(struct xfrm_state *xs,
>> +                                       u32 *mykey, u32 *mysalt)
>> +{
>> +       struct net_device *dev = xs->xso.dev;
>> +       unsigned char *key_data;
>> +       char *alg_name = NULL;
>> +       char *aes_gcm_name = "rfc4106(gcm(aes))";
>> +       int key_len;
>> +
>> +       if (xs->aead) {
>> +               key_data = &xs->aead->alg_key[0];
>> +               key_len = xs->aead->alg_key_len;
>> +               alg_name = xs->aead->alg_name;
>> +       } else {
>> +               netdev_err(dev, "Unsupported IPsec algorithm\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (strcmp(alg_name, aes_gcm_name)) {
>> +               netdev_err(dev, "Unsupported IPsec algorithm - please use %s\n",
>> +                          aes_gcm_name);
>> +               return -EINVAL;
>> +       }
>> +
>> +       /* 160 accounts for 16 byte key and 4 byte salt */
>> +       if (key_len == 128) {
>> +               netdev_info(dev, "IPsec hw offload parameters missing 32 bit salt value\n");
>> +       } else if (key_len != 160) {
>> +               netdev_err(dev, "IPsec hw offload only supports keys up to 128 bits with a 32 bit salt\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       /* The key bytes come down in a bigendian array of bytes, and
>> +        * salt is always the last 4 bytes of the key array.
>> +        * We don't need to do any byteswapping.
>> +        */
>> +       memcpy(mykey, key_data, 16);
>> +       if (key_len == 160)
>> +               *mysalt = ((u32 *)key_data)[4];
>> +       else
>> +               *mysalt = 0;
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_add_sa - program device with a security association
>> + * @xs: pointer to transformer state struct
>> + **/
>> +static int ixgbe_ipsec_add_sa(struct xfrm_state *xs)
>> +{
>> +       struct net_device *dev = xs->xso.dev;
>> +       struct ixgbe_adapter *adapter = netdev_priv(dev);
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       int checked, match, first;
>> +       u32 sa_idx;
>> +       int ret;
>> +       int i;
>> +
>> +       if (xs->id.proto != IPPROTO_ESP && xs->id.proto != IPPROTO_AH) {
>> +               netdev_err(dev, "Unsupported protocol 0x%04x for ipsec offload\n",
>> +                          xs->id.proto);
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
>> +               struct rx_sa rsa;
>> +
>> +               if (xs->calg) {
>> +                       netdev_err(dev, "Compression offload not supported\n");
>> +                       return -EINVAL;
>> +               }
>> +
>> +               /* find the first unused index */
>> +               sa_idx = ixgbe_ipsec_find_empty_idx(adapter, true);
>> +               if (sa_idx < 0) {
>> +                       netdev_err(dev, "No space for SA in Rx table!\n");
>> +                       return -ENOSPC;
>> +               }
>> +
>> +               memset(&rsa, 0, sizeof(rsa));
>> +               rsa.used = true;
>> +               rsa.xs = xs;
>> +
>> +               if (rsa.xs->id.proto & IPPROTO_ESP)
>> +                       rsa.decrypt = xs->ealg || xs->aead;
>> +
>> +               /* get the key and salt */
>> +               ret = ixgbe_ipsec_parse_proto_keys(xs, rsa.key, &rsa.salt);
>> +               if (ret) {
>> +                       netdev_err(dev, "Failed to get key data for Rx SA table\n");
>> +                       return ret;
>> +               }
>> +
>> +               /* get ip for rx sa table */
>> +               if (xs->xso.flags & XFRM_OFFLOAD_IPV6)
>> +                       memcpy(rsa.ipaddr, &xs->id.daddr.a6, 16);
>> +               else
>> +                       memcpy(&rsa.ipaddr[3], &xs->id.daddr.a4, 4);
>> +
>> +               /* The HW does not have a 1:1 mapping from keys to IP addrs, so
>> +                * check for a matching IP addr entry in the table.  If the addr
>> +                * already exists, use it; else find an unused slot and add the
>> +                * addr.  If one does not exist and there are no unused table
>> +                * entries, fail the request.
>> +                */
>> +
>> +               /* Find an existing match or first not used, and stop looking
>> +                * after we've checked all we know we have.
>> +                */
>> +               checked = 0;
>> +               match = -1;
>> +               first = -1;
>> +               for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT &&
>> +                           (checked < adapter->num_rx_sa || first < 0); i++) {
>> +                       if (adapter->rx_ip_sa_tbl[i].used) {
>> +                               if (!memcmp(adapter->rx_ip_sa_tbl[i].ipaddr,
>> +                                           rsa.ipaddr, sizeof(rsa.ipaddr))) {
>> +                                       match = i;
>> +                                       break;
>> +                               }
>> +                               checked++;
>> +                       } else if (first < 0) {
>> +                               first = i;  /* track the first empty seen */
>> +                       }
>> +               }
>> +
>> +               if (adapter->num_rx_sa == 0)
>> +                       first = 0;
>> +
>> +               if (match >= 0) {
>> +                       /* addrs are the same, we should use this one */
>> +                       rsa.iptbl_ind = match;
>> +                       adapter->rx_ip_sa_tbl[match].ref_cnt++;
>> +
>> +               } else if (first >= 0) {
>> +                       /* no matches, but here's an empty slot */
>> +                       rsa.iptbl_ind = first;
>> +
>> +                       memcpy(adapter->rx_ip_sa_tbl[first].ipaddr,
>> +                              rsa.ipaddr, sizeof(rsa.ipaddr));
>> +                       adapter->rx_ip_sa_tbl[first].ref_cnt = 1;
>> +                       adapter->rx_ip_sa_tbl[first].used = true;
>> +
>> +                       ixgbe_ipsec_set_rx_ip(hw, rsa.iptbl_ind, rsa.ipaddr);
>> +
>> +               } else {
>> +                       /* no match and no empty slot */
>> +                       netdev_err(dev, "No space for SA in Rx IP SA table\n");
>> +                       memset(&rsa, 0, sizeof(rsa));
>> +                       return -ENOSPC;
>> +               }
>> +
>> +               rsa.mode = IXGBE_RXMOD_VALID;
>> +               if (rsa.xs->id.proto & IPPROTO_ESP)
>> +                       rsa.mode |= IXGBE_RXMOD_PROTO_ESP;
>> +               if (rsa.decrypt)
>> +                       rsa.mode |= IXGBE_RXMOD_DECRYPT;
>> +               if (rsa.xs->xso.flags & XFRM_OFFLOAD_IPV6)
>> +                       rsa.mode |= IXGBE_RXMOD_IPV6;
>> +
>> +               /* the preparations worked, so save the info */
>> +               memcpy(&adapter->rx_sa_tbl[sa_idx], &rsa, sizeof(rsa));
>> +
>> +               ixgbe_ipsec_set_rx_sa(hw, sa_idx, rsa.xs->id.spi, rsa.key,
>> +                                     rsa.salt, rsa.mode, rsa.iptbl_ind);
>> +               xs->xso.offload_handle = sa_idx + IXGBE_IPSEC_BASE_RX_INDEX;
>> +
>> +               adapter->num_rx_sa++;
>> +       } else {
>> +               struct tx_sa tsa;
>> +
>> +               /* find the first unused index */
>> +               sa_idx = ixgbe_ipsec_find_empty_idx(adapter, false);
>> +               if (sa_idx < 0) {
>> +                       netdev_err(dev, "No space for SA in Tx table\n");
>> +                       return -ENOSPC;
>> +               }
>> +
>> +               memset(&tsa, 0, sizeof(tsa));
>> +               tsa.used = true;
>> +               tsa.xs = xs;
>> +
>> +               if (xs->id.proto & IPPROTO_ESP)
>> +                       tsa.encrypt = xs->ealg || xs->aead;
>> +
>> +               ret = ixgbe_ipsec_parse_proto_keys(xs, tsa.key, &tsa.salt);
>> +               if (ret) {
>> +                       netdev_err(dev, "Failed to get key data for Tx SA table\n");
>> +                       memset(&tsa, 0, sizeof(tsa));
>> +                       return ret;
>> +               }
>> +
>> +               /* the preparations worked, so save the info */
>> +               memcpy(&adapter->tx_sa_tbl[sa_idx], &tsa, sizeof(tsa));
>> +
>> +               ixgbe_ipsec_set_tx_sa(hw, sa_idx, tsa.key, tsa.salt);
>> +
>> +               xs->xso.offload_handle = sa_idx + IXGBE_IPSEC_BASE_TX_INDEX;
>> +
>> +               adapter->num_tx_sa++;
>> +       }
>> +
>> +       /* enable the engine if not already warmed up */
>> +       if (!(adapter->flags2 & IXGBE_FLAG2_IPSEC_ENABLED)) {
>> +               ixgbe_ipsec_start_engine(adapter);
>> +               adapter->flags2 |= IXGBE_FLAG2_IPSEC_ENABLED;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_del_sa - clear out this specific SA
>> + * @xs: pointer to transformer state struct
>> + **/
>> +static void ixgbe_ipsec_del_sa(struct xfrm_state *xs)
>> +{
>> +       struct net_device *dev = xs->xso.dev;
>> +       struct ixgbe_adapter *adapter = netdev_priv(dev);
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       u32 zerobuf[4] = {0, 0, 0, 0};
>> +       u16 sa_idx;
>> +
>> +       if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
>> +               struct rx_sa *rsa;
>> +               u8 iptbl_ind;
>> +
>> +               sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_RX_INDEX;
>> +               rsa = &adapter->rx_sa_tbl[sa_idx];
>> +
>> +               if (!rsa->used) {
>> +                       netdev_err(dev, "Invalid Rx SA selected sa_idx=%d offload_handle=%lu\n",
>> +                                  sa_idx, xs->xso.offload_handle);
>> +                       return;
>> +               }
>> +
>> +               /* if the IP table entry is referenced by only this SA,
>> +                * i.e. ref_cnt is only 1, clear the IP table entry as well
>> +                */
>> +               iptbl_ind = rsa->iptbl_ind;
>> +               if (adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt > 0) {
>> +                       adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt--;
>> +
>> +                       if (!adapter->rx_ip_sa_tbl[iptbl_ind].ref_cnt) {
>> +                               memset(&adapter->rx_ip_sa_tbl[iptbl_ind], 0,
>> +                                      sizeof(struct rx_ip_sa));
>> +                               ixgbe_ipsec_set_rx_ip(hw, iptbl_ind, zerobuf);
>> +                       }
>> +               }
>> +
>> +               /* clear the SA table entry */
>> +               memset(rsa, 0, sizeof(struct rx_sa));
>> +               ixgbe_ipsec_set_rx_sa(hw, sa_idx, 0, zerobuf, 0, 0, 0);
>> +
>> +               adapter->num_rx_sa--;
>> +
>> +       } else {
>> +               sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
>> +
>> +               if (!adapter->tx_sa_tbl[sa_idx].used) {
>> +                       netdev_err(dev, "Invalid Tx SA selected sa_idx=%d offload_handle=%lu\n",
>> +                                  sa_idx, xs->xso.offload_handle);
>> +                       return;
>> +               }
>> +
>> +               /* clear the SA table entry */
>> +               memset(&adapter->tx_sa_tbl[sa_idx], 0, sizeof(struct tx_sa));
>> +               ixgbe_ipsec_set_tx_sa(hw, sa_idx, zerobuf, 0);
>> +
>> +               adapter->num_tx_sa--;
>> +       }
>> +
>> +       /* if there are no SAs left, stop the engine to save energy */
>> +       if (adapter->num_rx_sa == 0 && adapter->num_tx_sa == 0) {
>> +               adapter->flags2 &= ~IXGBE_FLAG2_IPSEC_ENABLED;
>> +               ixgbe_ipsec_stop_engine(adapter);
>> +       }
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_free - called by xfrm garbage collections
>> + * @xs: pointer to transformer state struct
>> + *
>> + * We don't have any garbage to collect, so we shouldn't even bother
>> + * implementing this function.
>> + **/
>> +static void ixgbe_ipsec_free(struct xfrm_state *xs)
>> +{
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_offload_ok - can this packet use the xfrm hw offload
>> + * @skb: current data packet
>> + * @xs: pointer to transformer state struct
>> + **/
>> +static bool ixgbe_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
>> +{
>> +       if (xs->props.family == AF_INET) {
>> +               /* Offload with IPv4 options is not supported yet */
>> +               if (ip_hdr(skb)->ihl > 5)
>> +                       return false;
>> +       } else {
>> +               /* Offload with IPv6 extension headers is not support yet */
>> +               if (ipv6_ext_hdr(ipv6_hdr(skb)->nexthdr))
>> +                       return false;
>> +       }
>> +
>> +       return true;
>> +}
>> +
>> +static const struct xfrmdev_ops ixgbe_xfrmdev_ops = {
>> +       .xdo_dev_state_add = ixgbe_ipsec_add_sa,
>> +       .xdo_dev_state_delete = ixgbe_ipsec_del_sa,
>> +       .xdo_dev_state_free = ixgbe_ipsec_free,
>> +       .xdo_dev_offload_ok = ixgbe_ipsec_offload_ok,
>> +};
>> +
>> +/**
>> + * ixgbe_ipsec_tx - setup Tx flags for ipsec offload
>> + * @tx_ring: outgoing context
>> + * @skb: current data packet
>> + * @itd: ipsec Tx data for later use in building context descriptor
>> + **/
>> +int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
>> +                  struct ixgbe_ipsec_tx_data *itd)
>> +{
>> +       struct ixgbe_adapter *adapter = netdev_priv(tx_ring->netdev);
>> +       struct xfrm_state *xs;
>> +       struct tx_sa *tsa;
>> +
>> +       /* TODO: not supporting IPv6 yet */
>> +       if (skb->protocol != htons(ETH_P_IP))
>> +               return 0;
>> +
> 
> This is a bit too simplistic, and ignores VLANs. You would probably be
> much better off moving this code into ixgbe_main and keeping it
> somewhere close to ixgbe_xmit_frame_ring. Also instead of passing the
> skb, you might look at passing the first tx_buffer_info structure for
> the xmit, it would already have ETH_P_IP, and it would have skipped
> over any VLAN tags if they were present.

I'll look at that - thanks.

> 
>> +       if ((ip_hdr(skb)->protocol != IPPROTO_ESP) &&
>> +           (ip_hdr(skb)->protocol != IPPROTO_AH))
>> +               return 0;
>> +
>> +       if (!skb->sp) {
>> +               netdev_err(tx_ring->netdev, "%s: no xfrm state skb->sp = %p encap=%d skb=%p\n",
>> +                          __func__, skb->sp, skb->encapsulation, skb);
>> +               return 0;
>> +       }
>> +
> 
> I would make this check the first check in this function. Also the
> warning doesn't make any sense, is the assumption that if you support
> IPsec offload you are going to offload all IPsec frames? Seems like
> this is something where you could have a mix of both offloaded and
> non-offloaded frames. In addition this could cause issues if a
> interface is just performing routing between two interfaces that are
> passing IPsec traffic.

Yep.

> 
>> +       if (!skb->sp->len) {
>> +               netdev_err(tx_ring->netdev, "%s: no xfrm state len = %d\n",
>> +                          __func__, skb->sp->len);
>> +               return 0;
>> +       }
>> +
>> +       xs = xfrm_input_state(skb);
>> +       if (!xs) {
>> +               netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs = %p\n",
>> +                          __func__, xs);
>> +               return 0;
>> +       }
>> +
>> +       itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
>> +       if (itd->sa_idx < 0 || itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
>> +               netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d\n",
>> +                          __func__, itd->sa_idx);
>> +               return 0;
>> +       }
> 
> Does sa_idx need to be a signed value? If not you might do a unsigned
> comparison of the value and avoid having to do two comparisons.

Good idea.

> 
>> +
>> +       tsa = &adapter->tx_sa_tbl[itd->sa_idx];
>> +       if (!tsa->used) {
>> +               netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
>> +                          __func__, itd->sa_idx);
>> +               return 0;
>> +       }
>> +
>> +       itd->flags = 0;
>> +       if (xs->id.proto == IPPROTO_ESP) {
>> +               itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP |
>> +                             IXGBE_ADVTXD_TUCMD_L4T_TCP;
> 
> So you should probably drop the TCP flag here. It might explain some
> of the issues you are having with TSO and checksum offloads being
> enabled. That bit should only be set if we are requesting a TCP
> checksum offload.
> 
>> +               if (skb->protocol == htons(ETH_P_IP))
>> +                       itd->flags |= IXGBE_ADVTXD_TUCMD_IPV4;
> 
> Same thing for this bit. It assumes you want an IPv4 checksum offload.
> Normally it is only set for TSO packets where we have to update the IP
> header checksum.
> 
>> +               itd->trailer_len = xs->props.trailer_len;
>> +       }
> 
> You might look at just combining the flags and trailer_len field into
> one field and be done with it. Then all you have to do is pre-shift
> the fields before you write them into the appropriate spots.
> 
>> +       if (tsa->encrypt)
>> +               itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN;
>> +
>> +       return 1;
>> +}
>> +
>> +/**
>> + * ixgbe_ipsec_process_rx_offload - decode ipsec bits from Rx descriptor
>> + * @rx_ring: receiving ring
>> + * @rx_desc: receive data descriptor
>> + * @skb: current data packet
>> + *
>> + * Determine if there was an ipsec encapsulation noticed, and if so set up
>> + * the resulting status for later in the receive stack.
>> + **/
>> +void ixgbe_ipsec_process_rx_offload(struct ixgbe_ring *rx_ring,
>> +                                   union ixgbe_adv_rx_desc *rx_desc,
>> +                                   struct sk_buff *skb)
>> +{
>> +       struct ixgbe_adapter *adapter = netdev_priv(rx_ring->netdev);
>> +       u16 pkt_info = le16_to_cpu(rx_desc->wb.lower.lo_dword.hs_rss.pkt_info);
>> +       u32 status_error = le32_to_cpu(rx_desc->wb.upper.status_error);
>> +       u16 ipsec_pkt_types = IXGBE_RXDADV_PKTTYPE_IPSEC_AH |
>> +                               IXGBE_RXDADV_PKTTYPE_IPSEC_ESP;
>> +       struct xfrm_offload *xo = NULL;
>> +       struct xfrm_state *xs = NULL;
>> +       struct iphdr *iph;
>> +       u32 crypto_status = 0;
>> +       u8 *c_hdr;
>> +       __be32 spi;
>> +       u8 proto;
>> +
> 
> Probably the first check here should be for the SECP bit in the
> descriptor meaning that there was a security encapsulation recognized.
> Without that check you could end up wasting a ton of cycles on IPsec
> packets that aren't even being offloaded.
> 
> You might even be able to get away with checking for the SECP bit
> being set instead of having to check the adapter->flag2 value that you
> do in later patches to determine if you are going to handle the ipsec
> offload in the Rx path.
> 
>> +       /* TODO: we're only handling ipv4 at the moment */
>> +       if (!(pkt_info & IXGBE_RXDADV_PKTTYPE_IPV4))
>> +               return;
>> +
>> +       /* look for our packet types */
>> +       if (!(pkt_info & ipsec_pkt_types))
>> +               return;
>> +
> 
> I suspect you only need this check, you probably don't need the IPv4
> check. You would need to test to verify, but if the hardware doesn't
> support IPv6 offload with these protocols there is a good chance the
> parser won't handle it either.
> 
> In addition you might want to go through and verify the upper limit as
> well.  Basically if you verified that at least one bit is set, you
> should probably verify that only one bit is set as well. Then you
> could turn your switch statement below into just an if statement with
> two cases.



> 
>> +       /* don't process this if no offload work was done */
>> +       if (!(status_error & IXGBE_RXDADV_STAT_SECP))
>> +               return;
>> +
>> +       iph = (struct iphdr *)(skb_mac_header(skb) + ETH_HLEN);
>> +       c_hdr = (u8 *)iph + iph->ihl * 4;
> 
> This code is assuming quite a bit as it assumes no VLANs are present
> and I think they could be. I believe you are currently handling this
> after eth_type_trans is already called, otherwise skb_mac_header
> wouldn't work since it wouldn't be populated. It might be better to do
> all this before eth_type_trans is called and instead just use
> skb->data. You may want to look at using something like
> __vlan_get_protocol or code similar.
> 
>> +       switch (pkt_info & ipsec_pkt_types) {
>> +       case IXGBE_RXDADV_PKTTYPE_IPSEC_AH:
>> +               spi = ((struct ip_auth_hdr *)c_hdr)->spi;
>> +               proto = IPPROTO_AH;
>> +               break;
>> +       case IXGBE_RXDADV_PKTTYPE_IPSEC_ESP:
>> +               spi = ((struct ip_esp_hdr *)c_hdr)->spi;
>> +               proto = IPPROTO_ESP;
>> +               break;
>> +       default:
>> +               spi = 0;
>> +               proto = 0;
>> +               break;
>> +       }
>> +
>> +       xs = ixgbe_ipsec_find_rx_state(adapter, iph->daddr, proto, spi);
>> +       if (unlikely(!xs))
>> +               return;
> 
> Once again this function will need to be updated since it is in the Rx
> hotpath it would be best to look at possibly having it generate a hash
> and then use a hash table instead of an array for the lookup.
> 
>> +
>> +       /* decode error bits, if any */
>> +       if ((status_error & IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) == IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) {
>> +               if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_AH)
>> +                       crypto_status = CRYPTO_TRANSPORT_AH_AUTH_FAILED;
>> +               else if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_ESP)
>> +                       crypto_status = CRYPTO_TRANSPORT_ESP_AUTH_FAILED;
>> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL) {
>> +               crypto_status = CRYPTO_INVALID_PROTOCOL;
>> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH) {
>> +               crypto_status = CRYPTO_INVALID_PACKET_SYNTAX;
>> +       } else {
>> +               crypto_status = CRYPTO_SUCCESS;
>> +       }
>> +
> 
> You might look at converting this into a look-up table since you have
> essentially 2 protocols (AH or ESP), and 4 possible return values that
> have to be translated int the final result value. A single 2
> dimensional 8 byte array should be enough to handle all the possible
> crypto_status return values.

Actually, if we throw the packet away earlier in the path when looking 
at the IXGBE_RXDADV_ERR_FRAME_ERR_MASK macro, we won't need to look for 
these error bits at all.

> 
>> +       skb->sp = secpath_dup(skb->sp);
>> +       if (unlikely(!skb->sp))
>> +               return;
>> +
> 
> It seems like this should be done as soon as you have found your
> transform state. Otherwise there isn't much point in decoding the
> status_error bits.
> 
>> +       skb->sp->xvec[skb->sp->len++] = xs;
>> +       skb->sp->olen++;
>> +       xo = xfrm_offload(skb);
>> +       xo->flags = CRYPTO_DONE;
>> +       xo->status = crypto_status;
>> +
>> +       if (xo->status == CRYPTO_SUCCESS)
>> +               rx_ring->rx_stats.ipsec_offloads++;
>> +       else
>> +               rx_ring->rx_stats.ipsec_offload_faileds++;
> 
> You might rename offload_faileds to offload_errors, since the word
> "failed" isn't really something to pluralize.

I'm trying to decide if I want to keep these stats around at all.

> 
>> +}
>> +
>> +/**
>> + * ixgbe_init_ipsec_offload - initialize security registers for IPSec operation
>> + * @adapter: board private structure
>> + **/
>> +void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter)
>> +{
>> +       // TODO: enable and support checksum offload with ESP
>> +       //netdev_features_t esp_feats = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM;
>> +       netdev_features_t esp_feats = NETIF_F_HW_ESP;
>> +       size_t size;
>> +
>> +       /* allocate SA tables for rx, tx, and ip table */
>> +       size = sizeof(struct rx_sa) * IXGBE_IPSEC_MAX_SA_COUNT;
>> +       adapter->rx_sa_tbl = kzalloc(size, GFP_KERNEL);
>> +       if (!adapter->rx_sa_tbl)
>> +               goto err;
>> +
>> +       size = sizeof(struct tx_sa) * IXGBE_IPSEC_MAX_SA_COUNT;
>> +       adapter->tx_sa_tbl = kzalloc(size, GFP_KERNEL);
>> +       if (!adapter->tx_sa_tbl)
>> +               goto err;
>> +
>> +       size = sizeof(struct rx_ip_sa) * IXGBE_IPSEC_MAX_RX_IP_COUNT;
>> +       adapter->rx_ip_sa_tbl = kzalloc(size, GFP_KERNEL);
>> +       if (!adapter->rx_ip_sa_tbl)
>> +               goto err;
>> +
>> +       adapter->num_rx_sa = 0;
>> +       adapter->num_tx_sa = 0;
>> +       ixgbe_ipsec_clear_hw_tables(adapter);
>> +       ixgbe_ipsec_stop_engine(adapter);
>> +
>> +       adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops;
>> +       adapter->netdev->features |= esp_feats;
>> +       adapter->netdev->hw_enc_features |= esp_feats;
>> +
>> +       return;
>> +
>> +err:
>> +       kfree(adapter->rx_ip_sa_tbl);
>> +       kfree(adapter->rx_sa_tbl);
>> +       kfree(adapter->tx_sa_tbl);
>> +       netdev_err(adapter->netdev, "Unable to allocate memory for SA tables");
>> +}
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>> new file mode 100644
>> index 0000000..265950d
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>> @@ -0,0 +1,80 @@
>> +/*******************************************************************************
>> +
>> +  Intel 10 Gigabit PCI Express Linux driver
>> +  Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
>> +
>> +  This program is free software; you can redistribute it and/or modify it
>> +  under the terms and conditions of the GNU General Public License,
>> +  version 2, as published by the Free Software Foundation.
>> +
>> +  This program is distributed in the hope it will be useful, but WITHOUT
>> +  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> +  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> +  more details.
>> +
>> +  You should have received a copy of the GNU General Public License along with
>> +  this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +  The full GNU General Public License is included in this distribution in
>> +  the file called "COPYING".
>> +
>> +  Contact Information:
>> +  Linux NICS <linux.nics@intel.com>
>> +  e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
>> +  Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
>> +
>> +*******************************************************************************/
>> +
>> +#ifndef _IXGBE_IPSEC_H_
>> +#define _IXGBE_IPSEC_H_
>> +
>> +#define IXGBE_IPSEC_MAX_SA_COUNT       1024
>> +#define IXGBE_IPSEC_MAX_RX_IP_COUNT    128
>> +#define IXGBE_IPSEC_BASE_RX_INDEX      IXGBE_IPSEC_MAX_SA_COUNT
>> +#define IXGBE_IPSEC_BASE_TX_INDEX      (IXGBE_IPSEC_MAX_SA_COUNT * 2)
>> +
>> +#define IXGBE_RXTXIDX_IPS_EN           0x00000001
>> +#define IXGBE_RXIDX_TBL_MASK           0x00000006
>> +#define IXGBE_RXIDX_TBL_IP             0x00000002
>> +#define IXGBE_RXIDX_TBL_SPI            0x00000004
>> +#define IXGBE_RXIDX_TBL_KEY            0x00000006
>> +#define IXGBE_RXTXIDX_IDX_MASK         0x00001ff8
>> +#define IXGBE_RXTXIDX_IDX_READ         0x40000000
>> +#define IXGBE_RXTXIDX_IDX_WRITE                0x80000000
>> +
>> +#define IXGBE_RXMOD_VALID              0x00000001
>> +#define IXGBE_RXMOD_PROTO_ESP          0x00000004
>> +#define IXGBE_RXMOD_DECRYPT            0x00000008
>> +#define IXGBE_RXMOD_IPV6               0x00000010
>> +
>> +struct rx_sa {
>> +       struct xfrm_state *xs;
>> +       u32 ipaddr[4];
>> +       u32 key[4];
>> +       u32 salt;
>> +       u32 mode;
>> +       u8  iptbl_ind;
>> +       bool used;
>> +       bool decrypt;
>> +};
>> +
>> +struct rx_ip_sa {
>> +       u32 ipaddr[4];
>> +       u32 ref_cnt;
>> +       bool used;
>> +};
>> +
>> +struct tx_sa {
>> +       struct xfrm_state *xs;
>> +       u32 key[4];
>> +       u32 salt;
>> +       bool encrypt;
>> +       bool used;
>> +};
>> +
>> +struct ixgbe_ipsec_tx_data {
>> +       u32 flags;
>> +       u16 trailer_len;
>> +       u16 sa_idx;
>> +};
>> +#endif /* _IXGBE_IPSEC_H_ */
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan at osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 3/9] ixgbe: add the ipsec support code files

[Alexander Duyck]

On Sat, Nov 18, 2017 at 12:18 PM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> Again, thanks for the review on this early code.  I have a few responses
> below.
>
> sln

No problem. It is a nice break from working on macvlan code where
ixgbe seems to have more "undocumented features" than I know what to
do with.. :-)

I had a couple responses to your responses included them inline below
and snipped the portions of the patch I think we are in agreement on.

> On 11/17/2017 1:30 PM, Alexander Duyck wrote:
>>
>> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
>> <shannon.nelson@oracle.com> wrote:
>>>
>>> This adds the ipsec support code into the driver, but doesn't yet
>>> include it into the compile.
>>>
>>> It is possible that the .h file contents could simply go into
>>> the ixgbe_type.h and ixgbe.h files.
>>>
>>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>>
>>
>> Generally I really hate it when code is just thrown in like this as it
>> is really difficult to review and/or debug. I would really much prefer
>> to have this code added as it is needed for specific functionality.
>>
>>> ---
>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 1133
>>> ++++++++++++++++++++++++
>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h |   80 ++
>>>   2 files changed, 1213 insertions(+)
>>>   create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>   create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>> new file mode 100644
>>> index 0000000..177b915
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>> @@ -0,0 +1,1133 @@
>>>

<snip>

>>> +
>>> +       /* decode error bits, if any */
>>> +       if ((status_error & IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) ==
>>> IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) {
>>> +               if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_AH)
>>> +                       crypto_status = CRYPTO_TRANSPORT_AH_AUTH_FAILED;
>>> +               else if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_ESP)
>>> +                       crypto_status = CRYPTO_TRANSPORT_ESP_AUTH_FAILED;
>>> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL) {
>>> +               crypto_status = CRYPTO_INVALID_PROTOCOL;
>>> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH) {
>>> +               crypto_status = CRYPTO_INVALID_PACKET_SYNTAX;
>>> +       } else {
>>> +               crypto_status = CRYPTO_SUCCESS;
>>> +       }
>>> +
>>
>>
>> You might look at converting this into a look-up table since you have
>> essentially 2 protocols (AH or ESP), and 4 possible return values that
>> have to be translated int the final result value. A single 2
>> dimensional 8 byte array should be enough to handle all the possible
>> crypto_status return values.
>
>
> Actually, if we throw the packet away earlier in the path when looking at
> the IXGBE_RXDADV_ERR_FRAME_ERR_MASK macro, we won't need to look for these
> error bits at all.

The question is if it is valid to throw away the frame or if we should
handle this like we handle a checksum error and just pass it up the
stack to be processed. I assume if we have an error we don't decode
the frame. If so we could probably pass the frame up with no offloads
at all and let the stack deal with it all. That way if there end up
being any false bugs in the hardware we don't end up dropping valid
frames.

>>
>>> +       skb->sp = secpath_dup(skb->sp);
>>> +       if (unlikely(!skb->sp))
>>> +               return;
>>> +
>>
>>
>> It seems like this should be done as soon as you have found your
>> transform state. Otherwise there isn't much point in decoding the
>> status_error bits.
>>
>>> +       skb->sp->xvec[skb->sp->len++] = xs;
>>> +       skb->sp->olen++;
>>> +       xo = xfrm_offload(skb);
>>> +       xo->flags = CRYPTO_DONE;
>>> +       xo->status = crypto_status;
>>> +
>>> +       if (xo->status == CRYPTO_SUCCESS)
>>> +               rx_ring->rx_stats.ipsec_offloads++;
>>> +       else
>>> +               rx_ring->rx_stats.ipsec_offload_faileds++;
>>
>>
>> You might rename offload_faileds to offload_errors, since the word
>> "failed" isn't really something to pluralize.
>
>
> I'm trying to decide if I want to keep these stats around at all.

I'd say the stats are useful so they could probably be kept. If
nothing else they tell you if the offload is on, and if it is working
or not.

[Intel-wired-lan] [RFC PATCH jkirsher/next-queue 3/9] ixgbe: add the ipsec support code files

[Shannon Nelson]

On 11/18/2017 4:55 PM, Alexander Duyck wrote:
> On Sat, Nov 18, 2017 at 12:18 PM, Shannon Nelson
>> On 11/17/2017 1:30 PM, Alexander Duyck wrote:
>>>
>>> On Thu, Nov 16, 2017 at 11:54 AM, Shannon Nelson
>>> <shannon.nelson@oracle.com> wrote:
>>>>
>>>> This adds the ipsec support code into the driver, but doesn't yet
>>>> include it into the compile.
>>>>
>>>> It is possible that the .h file contents could simply go into
>>>> the ixgbe_type.h and ixgbe.h files.
>>>>
>>>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>>>
>>>
>>> Generally I really hate it when code is just thrown in like this as it
>>> is really difficult to review and/or debug. I would really much prefer
>>> to have this code added as it is needed for specific functionality.
>>>
>>>> ---
>>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 1133
>>>> ++++++++++++++++++++++++
>>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h |   80 ++
>>>>    2 files changed, 1213 insertions(+)
>>>>    create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>>    create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>> new file mode 100644
>>>> index 0000000..177b915
>>>> --- /dev/null
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>> @@ -0,0 +1,1133 @@
>>>>
> 
> <snip>
> 
>>>> +
>>>> +       /* decode error bits, if any */
>>>> +       if ((status_error & IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) ==
>>>> IXGBE_RXDADV_ERR_IPSEC_AUTH_FAILED) {
>>>> +               if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_AH)
>>>> +                       crypto_status = CRYPTO_TRANSPORT_AH_AUTH_FAILED;
>>>> +               else if (pkt_info & IXGBE_RXDADV_PKTTYPE_IPSEC_ESP)
>>>> +                       crypto_status = CRYPTO_TRANSPORT_ESP_AUTH_FAILED;
>>>> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_PROTOCOL) {
>>>> +               crypto_status = CRYPTO_INVALID_PROTOCOL;
>>>> +       } else if (status_error & IXGBE_RXDADV_ERR_IPSEC_INV_LENGTH) {
>>>> +               crypto_status = CRYPTO_INVALID_PACKET_SYNTAX;
>>>> +       } else {
>>>> +               crypto_status = CRYPTO_SUCCESS;
>>>> +       }
>>>> +
>>>
>>>
>>> You might look at converting this into a look-up table since you have
>>> essentially 2 protocols (AH or ESP), and 4 possible return values that
>>> have to be translated int the final result value. A single 2
>>> dimensional 8 byte array should be enough to handle all the possible
>>> crypto_status return values.
>>
>>
>> Actually, if we throw the packet away earlier in the path when looking at
>> the IXGBE_RXDADV_ERR_FRAME_ERR_MASK macro, we won't need to look for these
>> error bits at all.
> 
> The question is if it is valid to throw away the frame or if we should
> handle this like we handle a checksum error and just pass it up the
> stack to be processed. I assume if we have an error we don't decode
> the frame. If so we could probably pass the frame up with no offloads
> at all and let the stack deal with it all. That way if there end up
> being any false bugs in the hardware we don't end up dropping valid
> frames.

I need to double check what the XFRM code expects and work from there. 
Steffen has suggested that packets with errors from the chip shouldn't 
be passed up the stack.  Given that, I'm not sure why we would bother 
with having error values here, so just dropping the packets seems to be 
the thing to do.

> 
>>>
>>>> +       skb->sp = secpath_dup(skb->sp);
>>>> +       if (unlikely(!skb->sp))
>>>> +               return;
>>>> +
>>>
>>>
>>> It seems like this should be done as soon as you have found your
>>> transform state. Otherwise there isn't much point in decoding the
>>> status_error bits.
>>>
>>>> +       skb->sp->xvec[skb->sp->len++] = xs;
>>>> +       skb->sp->olen++;
>>>> +       xo = xfrm_offload(skb);
>>>> +       xo->flags = CRYPTO_DONE;
>>>> +       xo->status = crypto_status;
>>>> +
>>>> +       if (xo->status == CRYPTO_SUCCESS)
>>>> +               rx_ring->rx_stats.ipsec_offloads++;
>>>> +       else
>>>> +               rx_ring->rx_stats.ipsec_offload_faileds++;
>>>
>>>
>>> You might rename offload_faileds to offload_errors, since the word
>>> "failed" isn't really something to pluralize.
>>
>>
>> I'm trying to decide if I want to keep these stats around at all.
> 
> I'd say the stats are useful so they could probably be kept. If
> nothing else they tell you if the offload is on, and if it is working
> or not.

Maybe I'll fix it so that the stats aren't printed in the ethtool output 
if there are no current ipsec offloads?  Or maybe I'll combine them into 
a single pair of offloads and faileds counts rather than a pair for 
every queue - this might be better.

sln

>