| Message ID | 20201116023140.28975-1-tseewald@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxbgb4: Fix build failure when CHELSIO_TLS_DEVICE=n | expand |
On Sun, 15 Nov 2020 20:31:40 -0600 Tom Seewald wrote: > After commit 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > building the kernel with CHELSIO_T4=y and CHELSIO_TLS_DEVICE=n results > in the following error: > > ld: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.o: in function > `cxgb_select_queue': > cxgb4_main.c:(.text+0x2dac): undefined reference to `tls_validate_xmit_skb' > > This is caused by cxgb_select_queue() calling cxgb4_is_ktls_skb() without > checking if CHELSIO_TLS_DEVICE=y. Fix this by calling cxgb4_is_ktls_skb() > only when this config option is enabled. > > Fixes: 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > Signed-off-by: Tom Seewald <tseewald@gmail.com> > --- > drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > index 7fd264a6d085..8e8783afd6df 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > @@ -1176,7 +1176,9 @@ static u16 cxgb_select_queue(struct net_device *dev, struct sk_buff *skb, > txq = netdev_pick_tx(dev, skb, sb_dev); > if (xfrm_offload(skb) || is_ptp_enabled(skb, dev) || > skb->encapsulation || > - cxgb4_is_ktls_skb(skb) || > +#if IS_ENABLED(CONFIG_CHELSIO_TLS_DEVICE) > + cxgb4_is_ktls_skb(skb) || > +#endif /* CHELSIO_TLS_DEVICE */ > (proto != IPPROTO_TCP && proto != IPPROTO_UDP)) > txq = txq % pi->nqsets; > The tls header already tries to solve this issue, it just does it poorly. This is a better fix: diff --git a/include/net/tls.h b/include/net/tls.h index baf1e99d8193..2ff3f4f7954a 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -441,11 +441,11 @@ struct sk_buff * tls_validate_xmit_skb(struct sock *sk, struct net_device *dev, struct sk_buff *skb); static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk) { -#ifdef CONFIG_SOCK_VALIDATE_XMIT +#ifdef CONFIG_TLS_DEVICE return sk_fullsock(sk) && (smp_load_acquire(&sk->sk_validate_xmit_skb) == &tls_validate_xmit_skb); #else return false; Please test this and submit if it indeed solves the problem. Thanks!
On Tue, Nov 17, 2020 at 4:26 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Sun, 15 Nov 2020 20:31:40 -0600 Tom Seewald wrote: > > After commit 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > > building the kernel with CHELSIO_T4=y and CHELSIO_TLS_DEVICE=n results > > in the following error: > > > > ld: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.o: in function > > `cxgb_select_queue': > > cxgb4_main.c:(.text+0x2dac): undefined reference to `tls_validate_xmit_skb' > > > > This is caused by cxgb_select_queue() calling cxgb4_is_ktls_skb() without > > checking if CHELSIO_TLS_DEVICE=y. Fix this by calling cxgb4_is_ktls_skb() > > only when this config option is enabled. > > > > Fixes: 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > --- > > drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > index 7fd264a6d085..8e8783afd6df 100644 > > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > @@ -1176,7 +1176,9 @@ static u16 cxgb_select_queue(struct net_device *dev, struct sk_buff *skb, > > txq = netdev_pick_tx(dev, skb, sb_dev); > > if (xfrm_offload(skb) || is_ptp_enabled(skb, dev) || > > skb->encapsulation || > > - cxgb4_is_ktls_skb(skb) || > > +#if IS_ENABLED(CONFIG_CHELSIO_TLS_DEVICE) > > + cxgb4_is_ktls_skb(skb) || > > +#endif /* CHELSIO_TLS_DEVICE */ > > (proto != IPPROTO_TCP && proto != IPPROTO_UDP)) > > txq = txq % pi->nqsets; > > > > The tls header already tries to solve this issue, it just does it > poorly. This is a better fix: > > diff --git a/include/net/tls.h b/include/net/tls.h > index baf1e99d8193..2ff3f4f7954a 100644 > --- a/include/net/tls.h > +++ b/include/net/tls.h > @@ -441,11 +441,11 @@ struct sk_buff * > tls_validate_xmit_skb(struct sock *sk, struct net_device *dev, > struct sk_buff *skb); > > static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk) > { > -#ifdef CONFIG_SOCK_VALIDATE_XMIT > +#ifdef CONFIG_TLS_DEVICE > return sk_fullsock(sk) && > (smp_load_acquire(&sk->sk_validate_xmit_skb) == > &tls_validate_xmit_skb); > #else > return false; > > > Please test this and submit if it indeed solves the problem. > > Thanks! Hi Jakub, Thanks for the reply, unfortunately that patch does not resolve the issue, I still get the same error as before. After looking into this a bit further, the issue seems to be with CONFIG_TLS=m as everything works when CONFIG_TLS=y. I also see that there was a similar issue [1] reported by Intel's kbuild test robot where the cxgb4 driver isn't able to see the TLS symbols when CONFIG_TLS=m. [1] https://lkml.org/lkml/2020/8/7/601
On Wed, 18 Nov 2020 23:40:40 -0600 Tom Seewald wrote: > On Tue, Nov 17, 2020 at 4:26 PM Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Sun, 15 Nov 2020 20:31:40 -0600 Tom Seewald wrote: > > > After commit 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > > > building the kernel with CHELSIO_T4=y and CHELSIO_TLS_DEVICE=n results > > > in the following error: > > > > > > ld: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.o: in function > > > `cxgb_select_queue': > > > cxgb4_main.c:(.text+0x2dac): undefined reference to `tls_validate_xmit_skb' > > > > > > This is caused by cxgb_select_queue() calling cxgb4_is_ktls_skb() without > > > checking if CHELSIO_TLS_DEVICE=y. Fix this by calling cxgb4_is_ktls_skb() > > > only when this config option is enabled. > > > > > > Fixes: 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > > --- > > > drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > > index 7fd264a6d085..8e8783afd6df 100644 > > > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > > @@ -1176,7 +1176,9 @@ static u16 cxgb_select_queue(struct net_device *dev, struct sk_buff *skb, > > > txq = netdev_pick_tx(dev, skb, sb_dev); > > > if (xfrm_offload(skb) || is_ptp_enabled(skb, dev) || > > > skb->encapsulation || > > > - cxgb4_is_ktls_skb(skb) || > > > +#if IS_ENABLED(CONFIG_CHELSIO_TLS_DEVICE) > > > + cxgb4_is_ktls_skb(skb) || > > > +#endif /* CHELSIO_TLS_DEVICE */ > > > (proto != IPPROTO_TCP && proto != IPPROTO_UDP)) > > > txq = txq % pi->nqsets; > > > > > > > The tls header already tries to solve this issue, it just does it > > poorly. This is a better fix: > > > > diff --git a/include/net/tls.h b/include/net/tls.h > > index baf1e99d8193..2ff3f4f7954a 100644 > > --- a/include/net/tls.h > > +++ b/include/net/tls.h > > @@ -441,11 +441,11 @@ struct sk_buff * > > tls_validate_xmit_skb(struct sock *sk, struct net_device *dev, > > struct sk_buff *skb); > > > > static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk) > > { > > -#ifdef CONFIG_SOCK_VALIDATE_XMIT > > +#ifdef CONFIG_TLS_DEVICE > > return sk_fullsock(sk) && > > (smp_load_acquire(&sk->sk_validate_xmit_skb) == > > &tls_validate_xmit_skb); > > #else > > return false; > > > > > > Please test this and submit if it indeed solves the problem. > > > > Thanks! > > Hi Jakub, > > Thanks for the reply, unfortunately that patch does not resolve the > issue, I still get the same error as before. After looking into this a > bit further, the issue seems to be with CONFIG_TLS=m as everything > works when CONFIG_TLS=y. > > I also see that there was a similar issue [1] reported by Intel's > kbuild test robot where the cxgb4 driver isn't able to see the TLS > symbols when CONFIG_TLS=m. Interesting. Does your original patch solve the allyesconfig + TLS=m problem? Seems to me that CHELSIO_T4 should depend on (TLS || TLS=n), the CONFIG_CHELSIO_TLS_DEVICE has the dependency but AFAICT nothing prevents CONFIG_CHELSIO_TLS_DEVICE=m and CHELSIO_T4=y and cxgb4_main.c is under the latter.
On Thu, Nov 19, 2020 at 11:37 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 18 Nov 2020 23:40:40 -0600 Tom Seewald wrote: > > On Tue, Nov 17, 2020 at 4:26 PM Jakub Kicinski <kuba@kernel.org> wrote: > > > > > > On Sun, 15 Nov 2020 20:31:40 -0600 Tom Seewald wrote: > > > > After commit 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > > > > building the kernel with CHELSIO_T4=y and CHELSIO_TLS_DEVICE=n results > > > > in the following error: > > > > > > > > ld: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.o: in function > > > > `cxgb_select_queue': > > > > cxgb4_main.c:(.text+0x2dac): undefined reference to `tls_validate_xmit_skb' > > > > > > > > This is caused by cxgb_select_queue() calling cxgb4_is_ktls_skb() without > > > > checking if CHELSIO_TLS_DEVICE=y. Fix this by calling cxgb4_is_ktls_skb() > > > > only when this config option is enabled. > > > > > > > > Fixes: 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > > > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > > > --- > > > > drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > > > index 7fd264a6d085..8e8783afd6df 100644 > > > > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > > > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > > > > @@ -1176,7 +1176,9 @@ static u16 cxgb_select_queue(struct net_device *dev, struct sk_buff *skb, > > > > txq = netdev_pick_tx(dev, skb, sb_dev); > > > > if (xfrm_offload(skb) || is_ptp_enabled(skb, dev) || > > > > skb->encapsulation || > > > > - cxgb4_is_ktls_skb(skb) || > > > > +#if IS_ENABLED(CONFIG_CHELSIO_TLS_DEVICE) > > > > + cxgb4_is_ktls_skb(skb) || > > > > +#endif /* CHELSIO_TLS_DEVICE */ > > > > (proto != IPPROTO_TCP && proto != IPPROTO_UDP)) > > > > txq = txq % pi->nqsets; > > > > > > > > > > The tls header already tries to solve this issue, it just does it > > > poorly. This is a better fix: > > > > > > diff --git a/include/net/tls.h b/include/net/tls.h > > > index baf1e99d8193..2ff3f4f7954a 100644 > > > --- a/include/net/tls.h > > > +++ b/include/net/tls.h > > > @@ -441,11 +441,11 @@ struct sk_buff * > > > tls_validate_xmit_skb(struct sock *sk, struct net_device *dev, > > > struct sk_buff *skb); > > > > > > static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk) > > > { > > > -#ifdef CONFIG_SOCK_VALIDATE_XMIT > > > +#ifdef CONFIG_TLS_DEVICE > > > return sk_fullsock(sk) && > > > (smp_load_acquire(&sk->sk_validate_xmit_skb) == > > > &tls_validate_xmit_skb); > > > #else > > > return false; > > > > > > > > > Please test this and submit if it indeed solves the problem. > > > > > > Thanks! > > > > Hi Jakub, > > > > Thanks for the reply, unfortunately that patch does not resolve the > > issue, I still get the same error as before. After looking into this a > > bit further, the issue seems to be with CONFIG_TLS=m as everything > > works when CONFIG_TLS=y. > > > > I also see that there was a similar issue [1] reported by Intel's > > kbuild test robot where the cxgb4 driver isn't able to see the TLS > > symbols when CONFIG_TLS=m. > > Interesting. Does your original patch solve the allyesconfig + TLS=m > problem? No it does not solve it, my original patch was incorrect and should not be applied. It only masks the issue when using my specific kernel config. > Seems to me that CHELSIO_T4 should depend on (TLS || TLS=n), the > CONFIG_CHELSIO_TLS_DEVICE has the dependency but AFAICT nothing prevents > CONFIG_CHELSIO_TLS_DEVICE=m and CHELSIO_T4=y and cxgb4_main.c is under > the latter. You are right, adding (TLS || TLS=n) for CHELSIO_T4 resolves the build error I am encountering.
On Fri, 20 Nov 2020 00:54:00 -0600 Tom Seewald wrote: > > Seems to me that CHELSIO_T4 should depend on (TLS || TLS=n), the > > CONFIG_CHELSIO_TLS_DEVICE has the dependency but AFAICT nothing prevents > > CONFIG_CHELSIO_TLS_DEVICE=m and CHELSIO_T4=y and cxgb4_main.c is under > > the latter. > > You are right, adding (TLS || TLS=n) for CHELSIO_T4 resolves the build > error I am encountering. Great, please submit a patch.
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c index 7fd264a6d085..8e8783afd6df 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c @@ -1176,7 +1176,9 @@ static u16 cxgb_select_queue(struct net_device *dev, struct sk_buff *skb, txq = netdev_pick_tx(dev, skb, sb_dev); if (xfrm_offload(skb) || is_ptp_enabled(skb, dev) || skb->encapsulation || - cxgb4_is_ktls_skb(skb) || +#if IS_ENABLED(CONFIG_CHELSIO_TLS_DEVICE) + cxgb4_is_ktls_skb(skb) || +#endif /* CHELSIO_TLS_DEVICE */ (proto != IPPROTO_TCP && proto != IPPROTO_UDP)) txq = txq % pi->nqsets;
After commit 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") building the kernel with CHELSIO_T4=y and CHELSIO_TLS_DEVICE=n results in the following error: ld: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.o: in function `cxgb_select_queue': cxgb4_main.c:(.text+0x2dac): undefined reference to `tls_validate_xmit_skb' This is caused by cxgb_select_queue() calling cxgb4_is_ktls_skb() without checking if CHELSIO_TLS_DEVICE=y. Fix this by calling cxgb4_is_ktls_skb() only when this config option is enabled. Fixes: 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") Signed-off-by: Tom Seewald <tseewald@gmail.com> --- drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)