Hi, Guinian
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Sun GuinanX
> Sent: Thursday, October 31, 2019 7:32 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>;
> stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v6] net/ixgbe: fix macsec setting
>
> macsec setting is not valid when port is stopped.
> In order to make it valid, the patch changes the setting to where port is
> started.
>
> Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
> Cc: stable@dpdk.org
>
> Signed-off-by: Sun GuinanX <guinanx.sun@intel.com>
> 19 ++++ drivers/net/ixgbe/rte_pmd_ixgbe.c | 125 ++-----------------------
> 3 files changed, 175 insertions(+), 118 deletions(-)
>
> diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.c
> b/drivers/net/ixgbe/rte_pmd_ixgbe.c
> index 9514f2cf5..073fe1e23 100644
> --- a/drivers/net/ixgbe/rte_pmd_ixgbe.c
> +++ b/drivers/net/ixgbe/rte_pmd_ixgbe.c
> @@ -515,82 +515,19 @@ rte_pmd_ixgbe_set_vf_rate_limit(uint16_t port,
> uint16_t vf, int rte_pmd_ixgbe_macsec_enable(uint16_t port, uint8_t en,
> uint8_t rp) {
> - struct ixgbe_hw *hw;
> struct rte_eth_dev *dev;
> - uint32_t ctrl;
> + struct ixgbe_macsec_setting macsec_setting;
>
> RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
>
> dev = &rte_eth_devices[port];
>
> - if (!is_ixgbe_supported(dev))
> - return -ENOTSUP;
This driver type check of is_ixgbe_supported(dev), is very important for PRIVATE API function,
You can not delete it, Please add it back, rte_pmd_ixgbe_macsec_disable() also has this problem.
Although this patch has been merged, please commit fix patch for it, thanks!
> + macsec_setting.encrypt_en = en;
> + macsec_setting.replayprotect_en = rp;
>
> - hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + ixgbe_dev_macsec_setting_save(dev, &macsec_setting);
>
> - /* Stop the data paths */
> - if (ixgbe_disable_sec_rx_path(hw) != IXGBE_SUCCESS)
> - return -ENOTSUP;
@@ -2542,6 +2542,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
uint32_t *link_speeds;
struct ixgbe_tm_conf *tm_conf =
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+ struct ixgbe_macsec_setting *macsec_ctrl =
+ IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
PMD_INIT_FUNC_TRACE();
@@ -2794,6 +2796,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
*/
ixgbe_dev_link_update(dev, 0);
+ /* setup the macsec ctrl register */
+ ixgbe_dev_macsec_register_enable(dev, macsec_ctrl);
+
return 0;
error:
@@ -2825,6 +2830,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
PMD_INIT_FUNC_TRACE();
+ /* disable mecsec register */
+ ixgbe_dev_macsec_register_disable(dev);
+
rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
/* disable interrupts */
@@ -8816,6 +8824,147 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
return 0;
}
+void
+ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
+ struct ixgbe_macsec_setting *macsec_setting)
+{
+ struct ixgbe_macsec_setting *macsec =
+ IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
+
+ macsec->encrypt_en = macsec_setting->encrypt_en;
+ macsec->replayprotect_en = macsec_setting->replayprotect_en;
+}
+
+void
+ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
+{
+ struct ixgbe_macsec_setting *macsec =
+ IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
+
+ macsec->encrypt_en = 0;
+ macsec->replayprotect_en = 0;
+}
+
+void
+ixgbe_dev_macsec_register_enable(struct rte_eth_dev *dev,
+ struct ixgbe_macsec_setting *macsec_setting)
+{
+ struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ uint32_t ctrl;
+ uint8_t en = macsec_setting->encrypt_en;
+ uint8_t rp = macsec_setting->replayprotect_en;
+
+ /**
+ * Workaround:
+ * As no ixgbe_disable_sec_rx_path equivalent is
+ * implemented for tx in the base code, and we are
+ * not allowed to modify the base code in DPDK, so
+ * just call the hand-written one directly for now.
+ * The hardware support has been checked by
+ * ixgbe_disable_sec_rx_path().
+ */
+ ixgbe_disable_sec_tx_path_generic(hw);
+
+ /* Enable Ethernet CRC (required by MACsec offload) */
+ ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+ ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+ IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+ /* Enable the TX and RX crypto engines */
+ ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+ ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+ IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+ ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+ ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+ IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+ ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+ ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+ ctrl |= 0x3;
+ IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+ /* Enable SA lookup */
+ ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+ ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+ ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
+ IXGBE_LSECTXCTRL_AUTH;
+ ctrl |= IXGBE_LSECTXCTRL_AISCI;
+ ctrl &= ~IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+ ctrl |= IXGBE_MACSEC_PNTHRSH & IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+ IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
+
+ ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
+ ctrl &= ~IXGBE_LSECRXCTRL_EN_MASK;
+ ctrl |= IXGBE_LSECRXCTRL_STRICT << IXGBE_LSECRXCTRL_EN_SHIFT;
+ ctrl &= ~IXGBE_LSECRXCTRL_PLSH;
+ if (rp)
+ ctrl |= IXGBE_LSECRXCTRL_RP;
+ else
+ ctrl &= ~IXGBE_LSECRXCTRL_RP;
+ IXGBE_WRITE_REG(hw, IXGBE_LSECRXCTRL, ctrl);
+
+ /* Start the data paths */
+ ixgbe_enable_sec_rx_path(hw);
+ /**
+ * Workaround:
+ * As no ixgbe_enable_sec_rx_path equivalent is
+ * implemented for tx in the base code, and we are
+ * not allowed to modify the base code in DPDK, so
+ * just call the hand-written one directly for now.
+ */
+ ixgbe_enable_sec_tx_path_generic(hw);
+}
+
+void
+ixgbe_dev_macsec_register_disable(struct rte_eth_dev *dev)
+{
+ struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ uint32_t ctrl;
+
+ /**
+ * Workaround:
+ * As no ixgbe_disable_sec_rx_path equivalent is
+ * implemented for tx in the base code, and we are
+ * not allowed to modify the base code in DPDK, so
+ * just call the hand-written one directly for now.
+ * The hardware support has been checked by
+ * ixgbe_disable_sec_rx_path().
+ */
+ ixgbe_disable_sec_tx_path_generic(hw);
+
+ /* Disable the TX and RX crypto engines */
+ ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+ ctrl |= IXGBE_SECTXCTRL_SECTX_DIS;
+ IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+ ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+ ctrl |= IXGBE_SECRXCTRL_SECRX_DIS;
+ IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+ /* Disable SA lookup */
+ ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+ ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+ ctrl |= IXGBE_LSECTXCTRL_DISABLE;
+ IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
+
+ ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
+ ctrl &= ~IXGBE_LSECRXCTRL_EN_MASK;
+ ctrl |= IXGBE_LSECRXCTRL_DISABLE << IXGBE_LSECRXCTRL_EN_SHIFT;
+ IXGBE_WRITE_REG(hw, IXGBE_LSECRXCTRL, ctrl);
+
+ /* Start the data paths */
+ ixgbe_enable_sec_rx_path(hw);
+ /**
+ * Workaround:
+ * As no ixgbe_enable_sec_rx_path equivalent is
+ * implemented for tx in the base code, and we are
+ * not allowed to modify the base code in DPDK, so
+ * just call the hand-written one directly for now.
+ */
+ ixgbe_enable_sec_tx_path_generic(hw);
+}
+
RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd);
RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic | vfio-pci");
@@ -365,6 +365,11 @@ struct rte_flow {
void *rule;
};
+struct ixgbe_macsec_setting {
+ uint8_t encrypt_en;
+ uint8_t replayprotect_en;
+};
+
/*
* Statistics counters collected by the MACsec
*/
@@ -471,6 +476,7 @@ struct ixgbe_adapter {
struct ixgbe_hw hw;
struct ixgbe_hw_stats stats;
struct ixgbe_macsec_stats macsec_stats;
+ struct ixgbe_macsec_setting macsec_setting;
struct ixgbe_hw_fdir_info fdir;
struct ixgbe_interrupt intr;
struct ixgbe_stat_mapping_registers stat_mappings;
@@ -523,6 +529,9 @@ int ixgbe_vf_representor_uninit(struct rte_eth_dev *ethdev);
#define IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(adapter) \
(&((struct ixgbe_adapter *)adapter)->macsec_stats)
+#define IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(adapter) \
+ (&((struct ixgbe_adapter *)adapter)->macsec_setting)
+
#define IXGBE_DEV_PRIVATE_TO_INTR(adapter) \
(&((struct ixgbe_adapter *)adapter)->intr)
@@ -741,6 +750,16 @@ int ixgbe_action_rss_same(const struct rte_flow_action_rss *comp,
int ixgbe_config_rss_filter(struct rte_eth_dev *dev,
struct ixgbe_rte_flow_rss_conf *conf, bool add);
+void ixgbe_dev_macsec_register_enable(struct rte_eth_dev *dev,
+ struct ixgbe_macsec_setting *macsec_setting);
+
+void ixgbe_dev_macsec_register_disable(struct rte_eth_dev *dev);
+
+void ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
+ struct ixgbe_macsec_setting *macsec_setting);
+
+void ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev);
+
static inline int
ixgbe_ethertype_filter_lookup(struct ixgbe_filter_info *filter_info,
uint16_t ethertype)
@@ -515,82 +515,19 @@ rte_pmd_ixgbe_set_vf_rate_limit(uint16_t port, uint16_t vf,
int
rte_pmd_ixgbe_macsec_enable(uint16_t port, uint8_t en, uint8_t rp)
{
- struct ixgbe_hw *hw;
struct rte_eth_dev *dev;
- uint32_t ctrl;
+ struct ixgbe_macsec_setting macsec_setting;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
dev = &rte_eth_devices[port];
- if (!is_ixgbe_supported(dev))
- return -ENOTSUP;
+ macsec_setting.encrypt_en = en;
+ macsec_setting.replayprotect_en = rp;
- hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ ixgbe_dev_macsec_setting_save(dev, &macsec_setting);
- /* Stop the data paths */
- if (ixgbe_disable_sec_rx_path(hw) != IXGBE_SUCCESS)
- return -ENOTSUP;
- /**
- * Workaround:
- * As no ixgbe_disable_sec_rx_path equivalent is
- * implemented for tx in the base code, and we are
- * not allowed to modify the base code in DPDK, so
- * just call the hand-written one directly for now.
- * The hardware support has been checked by
- * ixgbe_disable_sec_rx_path().
- */
- ixgbe_disable_sec_tx_path_generic(hw);
-
- /* Enable Ethernet CRC (required by MACsec offload) */
- ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
- ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
- IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
-
- /* Enable the TX and RX crypto engines */
- ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
- ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
- IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
-
- ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
- ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
- IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
-
- ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
- ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
- ctrl |= 0x3;
- IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
-
- /* Enable SA lookup */
- ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
- ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
- ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
- IXGBE_LSECTXCTRL_AUTH;
- ctrl |= IXGBE_LSECTXCTRL_AISCI;
- ctrl &= ~IXGBE_LSECTXCTRL_PNTHRSH_MASK;
- ctrl |= IXGBE_MACSEC_PNTHRSH & IXGBE_LSECTXCTRL_PNTHRSH_MASK;
- IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
-
- ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
- ctrl &= ~IXGBE_LSECRXCTRL_EN_MASK;
- ctrl |= IXGBE_LSECRXCTRL_STRICT << IXGBE_LSECRXCTRL_EN_SHIFT;
- ctrl &= ~IXGBE_LSECRXCTRL_PLSH;
- if (rp)
- ctrl |= IXGBE_LSECRXCTRL_RP;
- else
- ctrl &= ~IXGBE_LSECRXCTRL_RP;
- IXGBE_WRITE_REG(hw, IXGBE_LSECRXCTRL, ctrl);
-
- /* Start the data paths */
- ixgbe_enable_sec_rx_path(hw);
- /**
- * Workaround:
- * As no ixgbe_enable_sec_rx_path equivalent is
- * implemented for tx in the base code, and we are
- * not allowed to modify the base code in DPDK, so
- * just call the hand-written one directly for now.
- */
- ixgbe_enable_sec_tx_path_generic(hw);
+ ixgbe_dev_macsec_register_enable(dev, &macsec_setting);
return 0;
}
@@ -598,63 +535,15 @@ rte_pmd_ixgbe_macsec_enable(uint16_t port, uint8_t en, uint8_t rp)
int
rte_pmd_ixgbe_macsec_disable(uint16_t port)
{
- struct ixgbe_hw *hw;
struct rte_eth_dev *dev;
- uint32_t ctrl;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
dev = &rte_eth_devices[port];
- if (!is_ixgbe_supported(dev))
- return -ENOTSUP;
+ ixgbe_dev_macsec_setting_reset(dev);
- hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-
- /* Stop the data paths */
- if (ixgbe_disable_sec_rx_path(hw) != IXGBE_SUCCESS)
- return -ENOTSUP;
- /**
- * Workaround:
- * As no ixgbe_disable_sec_rx_path equivalent is
- * implemented for tx in the base code, and we are
- * not allowed to modify the base code in DPDK, so
- * just call the hand-written one directly for now.
- * The hardware support has been checked by
- * ixgbe_disable_sec_rx_path().
- */
- ixgbe_disable_sec_tx_path_generic(hw);
-
- /* Disable the TX and RX crypto engines */
- ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
- ctrl |= IXGBE_SECTXCTRL_SECTX_DIS;
- IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
-
- ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
- ctrl |= IXGBE_SECRXCTRL_SECRX_DIS;
- IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
-
- /* Disable SA lookup */
- ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
- ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
- ctrl |= IXGBE_LSECTXCTRL_DISABLE;
- IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
-
- ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
- ctrl &= ~IXGBE_LSECRXCTRL_EN_MASK;
- ctrl |= IXGBE_LSECRXCTRL_DISABLE << IXGBE_LSECRXCTRL_EN_SHIFT;
- IXGBE_WRITE_REG(hw, IXGBE_LSECRXCTRL, ctrl);
-
- /* Start the data paths */
- ixgbe_enable_sec_rx_path(hw);
- /**
- * Workaround:
- * As no ixgbe_enable_sec_rx_path equivalent is
- * implemented for tx in the base code, and we are
- * not allowed to modify the base code in DPDK, so
- * just call the hand-written one directly for now.
- */
- ixgbe_enable_sec_tx_path_generic(hw);
+ ixgbe_dev_macsec_register_disable(dev);
return 0;
}