On Thu, Nov 19, 2020 at 11:37 AM Jakub Kicinski <k...@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 <k...@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 <tseew...@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.
