diff mbox series

[net-next] ixgbe: fix XFRM_ALGO dependency

Message ID 20181016100848.1329843-1-arnd@arndb.de
State New
Headers show
Series [net-next] ixgbe: fix XFRM_ALGO dependency | expand

Commit Message

Arnd Bergmann Oct. 16, 2018, 10:03 a.m. UTC 
When XFRM_ALGO is not enabled, the new igxge ipsec code produces a
link error:

drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.o: In function `ixgbe_ipsec_vf_add_sa':
ixgbe_ipsec.c:(.text+0x1266): undefined reference to `xfrm_aead_get_byname'

Simply selectin XFRM_ALGO from here causes circular dependencies, so
to fix it, we probably want this slightly more complex solution that is
similar to what other drivers with XFRM offload do:

A separate Kconfig symbol now controls whether we include the ipsec
offload code. To keep the old behavior, this is left as 'default y'. The
dependency in XFRM_OFFLOAD still causes a circular dependency but is
not actually needed because this symbol is not user visible, so removing
that dependency on top makes it all work.

Fixes: eda0333ac293 ("ixgbe: add VF IPsec management")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/net/ethernet/intel/Kconfig            | 10 ++++++++++
 drivers/net/ethernet/intel/ixgbe/Makefile     |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  8 ++++----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +++---
 net/xfrm/Kconfig                              |  1 -
 5 files changed, 18 insertions(+), 9 deletions(-)

-- 
2.18.0

Comments

Shannon Nelson Oct. 16, 2018, 4:35 p.m. UTC | #1 
On 10/16/2018 3:03 AM, Arnd Bergmann wrote:
> When XFRM_ALGO is not enabled, the new igxge ipsec code produces a

> link error:


s/igxge/ixgbe/

> 

> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.o: In function `ixgbe_ipsec_vf_add_sa':

> ixgbe_ipsec.c:(.text+0x1266): undefined reference to `xfrm_aead_get_byname'

> 

> Simply selectin XFRM_ALGO from here causes circular dependencies, so


s/selectin/selecting/

> to fix it, we probably want this slightly more complex solution that is

> similar to what other drivers with XFRM offload do:

> 

> A separate Kconfig symbol now controls whether we include the ipsec

> offload code. To keep the old behavior, this is left as 'default y'. The

> dependency in XFRM_OFFLOAD still causes a circular dependency but is

> not actually needed because this symbol is not user visible, so removing

> that dependency on top makes it all work.

> 

> Fixes: eda0333ac293 ("ixgbe: add VF IPsec management")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>   drivers/net/ethernet/intel/Kconfig            | 10 ++++++++++

>   drivers/net/ethernet/intel/ixgbe/Makefile     |  2 +-

>   drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  8 ++++----

>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +++---

>   net/xfrm/Kconfig                              |  1 -

>   5 files changed, 18 insertions(+), 9 deletions(-)

> 

> diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig

> index b542aba6f0e8..d1db382d8299 100644

> --- a/drivers/net/ethernet/intel/Kconfig

> +++ b/drivers/net/ethernet/intel/Kconfig

> @@ -194,6 +194,16 @@ config IXGBE_DCB

>   

>   	  If unsure, say N.

>   

> +config IXGBE_IPSEC

> +	bool "IPSec XFRM cryptography-offload accelaration"

> +	default n


remove this "default n" line?

> +	depends on IXGBE

> +	depends on XFRM_OFFLOAD

> +	default y

> +	select XFRM_ALGO

> +	---help---

> +	  Enable support for IPSec offload in ixgbe.ko

> +

>   config IXGBEVF

>   	tristate "Intel(R) 10GbE PCI Express Virtual Function Ethernet support"

>   	depends on PCI_MSI

> diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile

> index ca6b0c458e4a..4fb0d9e3f2da 100644

> --- a/drivers/net/ethernet/intel/ixgbe/Makefile

> +++ b/drivers/net/ethernet/intel/ixgbe/Makefile

> @@ -17,4 +17,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

> +ixgbe-$(CONFIG_IXGBE_IPSEC) += ixgbe_ipsec.o

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h

> index 7a7679e7be84..1d5d66436eac 100644

> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h

> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h

> @@ -770,9 +770,9 @@ struct ixgbe_adapter {

>   #define IXGBE_RSS_KEY_SIZE     40  /* size of RSS Hash Key in bytes */

>   	u32 *rss_key;

>   

> -#ifdef CONFIG_XFRM_OFFLOAD

> +#ifdef CONFIG_IXGBE_IPSEC

>   	struct ixgbe_ipsec *ipsec;

> -#endif /* CONFIG_XFRM_OFFLOAD */

> +#endif /* CONFIG_IXGBE_IPSEC */


You'll probably want to apply these changes to the ixgbevf code as well.

Cheers,
sln

>   

>   	/* AF_XDP zero-copy */

>   	struct xdp_umem **xsk_umems;

> @@ -1009,7 +1009,7 @@ 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

> +#ifdef CONFIG_IXGBE_IPSEC

>   void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter);

>   void ixgbe_stop_ipsec_offload(struct ixgbe_adapter *adapter);

>   void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter);

> @@ -1037,5 +1037,5 @@ static inline int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter,

>   					u32 *mbuf, u32 vf) { return -EACCES; }

>   static inline int ixgbe_ipsec_vf_del_sa(struct ixgbe_adapter *adapter,

>   					u32 *mbuf, u32 vf) { return -EACCES; }

> -#endif /* CONFIG_XFRM_OFFLOAD */

> +#endif /* CONFIG_IXGBE_IPSEC */

>   #endif /* _IXGBE_H_ */

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

> index 0049a2becd7e..113b38e0defb 100644

> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

> @@ -8694,7 +8694,7 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,

>   

>   #endif /* IXGBE_FCOE */

>   

> -#ifdef CONFIG_XFRM_OFFLOAD

> +#ifdef CONFIG_IXGBE_IPSEC

>   	if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx))

>   		goto out_drop;

>   #endif

> @@ -10190,7 +10190,7 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev,

>   	 * the TSO, so it's the exception.

>   	 */

>   	if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) {

> -#ifdef CONFIG_XFRM_OFFLOAD

> +#ifdef CONFIG_IXGBE_IPSEC

>   		if (!skb->sp)

>   #endif

>   			features &= ~NETIF_F_TSO;

> @@ -10883,7 +10883,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)

>   	if (hw->mac.type >= ixgbe_mac_82599EB)

>   		netdev->features |= NETIF_F_SCTP_CRC;

>   

> -#ifdef CONFIG_XFRM_OFFLOAD

> +#ifdef CONFIG_IXGBE_IPSEC

>   #define IXGBE_ESP_FEATURES	(NETIF_F_HW_ESP | \

>   				 NETIF_F_HW_ESP_TX_CSUM | \

>   				 NETIF_F_GSO_ESP)

> diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig

> index 4a9ee2d83158..140270a13d54 100644

> --- a/net/xfrm/Kconfig

> +++ b/net/xfrm/Kconfig

> @@ -8,7 +8,6 @@ config XFRM

>   

>   config XFRM_OFFLOAD

>          bool

> -       depends on XFRM

>   

>   config XFRM_ALGO

>   	tristate

>
Arnd Bergmann Oct. 17, 2018, 4:04 p.m. UTC | #2 
On Wed, Oct 17, 2018 at 5:53 PM Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> On Tue, 2018-10-16 at 09:35 -0700, Shannon Nelson wrote:

> > On 10/16/2018 3:03 AM, Arnd Bergmann wrote:

> > > A separate Kconfig symbol now controls whether we include the ipsec

> > > offload code. To keep the old behavior, this is left as 'default

> > > y'. The

> > > dependency in XFRM_OFFLOAD still causes a circular dependency but

> > > is

> > > not actually needed because this symbol is not user visible, so

> > > removing

> > > that dependency on top makes it all work.

> > >

> > > Fixes: eda0333ac293 ("ixgbe: add VF IPsec management")

> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

>

> I agree with Shannon's suggested changes.  Arnd, are you working on v2?

> Or would you like me to take care of it?


I was planning to respin it, but didn't get around to it yet, and will
be travelling for the next week, so I'd welcome if you can take over
from here. Shannon's comments all make sense to me as well.

> > > +config IXGBE_IPSEC

> > > +   bool "IPSec XFRM cryptography-offload accelaration"

> > > +   default n

> >

> > remove this "default n" line?


I meant for this to say "default y", as I said in the changelog,
but feel free to pick whichever default makes sense to you
make make the description match ;-)

Thanks,

       Arnd
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index b542aba6f0e8..d1db382d8299 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -194,6 +194,16 @@  config IXGBE_DCB
 
 	  If unsure, say N.
 
+config IXGBE_IPSEC
+	bool "IPSec XFRM cryptography-offload accelaration"
+	default n
+	depends on IXGBE
+	depends on XFRM_OFFLOAD
+	default y
+	select XFRM_ALGO
+	---help---
+	  Enable support for IPSec offload in ixgbe.ko
+
 config IXGBEVF
 	tristate "Intel(R) 10GbE PCI Express Virtual Function Ethernet support"
 	depends on PCI_MSI
diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile
index ca6b0c458e4a..4fb0d9e3f2da 100644
--- a/drivers/net/ethernet/intel/ixgbe/Makefile
+++ b/drivers/net/ethernet/intel/ixgbe/Makefile
@@ -17,4 +17,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
+ixgbe-$(CONFIG_IXGBE_IPSEC) += ixgbe_ipsec.o
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 7a7679e7be84..1d5d66436eac 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -770,9 +770,9 @@  struct ixgbe_adapter {
 #define IXGBE_RSS_KEY_SIZE     40  /* size of RSS Hash Key in bytes */
 	u32 *rss_key;
 
-#ifdef CONFIG_XFRM_OFFLOAD
+#ifdef CONFIG_IXGBE_IPSEC
 	struct ixgbe_ipsec *ipsec;
-#endif /* CONFIG_XFRM_OFFLOAD */
+#endif /* CONFIG_IXGBE_IPSEC */
 
 	/* AF_XDP zero-copy */
 	struct xdp_umem **xsk_umems;
@@ -1009,7 +1009,7 @@  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
+#ifdef CONFIG_IXGBE_IPSEC
 void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter);
 void ixgbe_stop_ipsec_offload(struct ixgbe_adapter *adapter);
 void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter);
@@ -1037,5 +1037,5 @@  static inline int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter,
 					u32 *mbuf, u32 vf) { return -EACCES; }
 static inline int ixgbe_ipsec_vf_del_sa(struct ixgbe_adapter *adapter,
 					u32 *mbuf, u32 vf) { return -EACCES; }
-#endif /* CONFIG_XFRM_OFFLOAD */
+#endif /* CONFIG_IXGBE_IPSEC */
 #endif /* _IXGBE_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0049a2becd7e..113b38e0defb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8694,7 +8694,7 @@  netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
 
 #endif /* IXGBE_FCOE */
 
-#ifdef CONFIG_XFRM_OFFLOAD
+#ifdef CONFIG_IXGBE_IPSEC
 	if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx))
 		goto out_drop;
 #endif
@@ -10190,7 +10190,7 @@  ixgbe_features_check(struct sk_buff *skb, struct net_device *dev,
 	 * the TSO, so it's the exception.
 	 */
 	if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) {
-#ifdef CONFIG_XFRM_OFFLOAD
+#ifdef CONFIG_IXGBE_IPSEC
 		if (!skb->sp)
 #endif
 			features &= ~NETIF_F_TSO;
@@ -10883,7 +10883,7 @@  static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (hw->mac.type >= ixgbe_mac_82599EB)
 		netdev->features |= NETIF_F_SCTP_CRC;
 
-#ifdef CONFIG_XFRM_OFFLOAD
+#ifdef CONFIG_IXGBE_IPSEC
 #define IXGBE_ESP_FEATURES	(NETIF_F_HW_ESP | \
 				 NETIF_F_HW_ESP_TX_CSUM | \
 				 NETIF_F_GSO_ESP)
diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index 4a9ee2d83158..140270a13d54 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -8,7 +8,6 @@  config XFRM
 
 config XFRM_OFFLOAD
        bool
-       depends on XFRM
 
 config XFRM_ALGO
 	tristate