| Message ID | 20190425165637.7924-2-jakub.kicinski@netronome.com |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | net/tls: small code cleanup | expand |
On Thu, 2019-04-25 at 09:56 -0700, Jakub Kicinski wrote: > Currently when CONFIG_TLS_DEVICE is set each time kTLS > connection is opened and the offload is not successful > (either because the underlying device doesn't support > it or e.g. it's tables are full) a rate limited error > will be printed to the logs. > > There is nothing wrong with failing TLS offload. SW > path will process the packets just fine, avoid the > noisy messages by demoting them from error to debug > level. > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> > Reviewed-by: Reviewed-by: Simon Horman <simon.horman@netronome.com> Did simon review this patch twice ;) ? please fixup "Reviewed-by:" Tag. and also i think it shouldn't be even debug, it should be silent. The device didn't advertise the feature, why would we print a debug message on every TLS connection ? > --- > net/tls/tls_device.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c > index 9f3bdbc1e593..9d67989ab7a7 100644 > --- a/net/tls/tls_device.c > +++ b/net/tls/tls_device.c > @@ -865,8 +865,8 @@ int tls_set_device_offload_rx(struct sock *sk, > struct tls_context *ctx) > } > > if (!(netdev->features & NETIF_F_HW_TLS_RX)) { > - pr_err_ratelimited("%s: netdev %s with no TLS > offload\n", > - __func__, netdev->name); > + pr_debug_ratelimited("%s: netdev %s with no TLS > offload\n", > + __func__, netdev->name); > rc = -ENOTSUPP; > goto release_netdev; > } > @@ -895,8 +895,8 @@ int tls_set_device_offload_rx(struct sock *sk, > struct tls_context *ctx) > &ctx->crypto_recv.info, > tcp_sk(sk)->copied_seq); > if (rc) { > - pr_err_ratelimited("%s: The netdev has refused to > offload this socket\n", > - __func__); > + pr_debug_ratelimited("%s: The netdev has refused to > offload this socket\n", > + __func__); > goto free_sw_resources; > } >
On Thu, 25 Apr 2019 18:37:19 +0000, Saeed Mahameed wrote: > On Thu, 2019-04-25 at 09:56 -0700, Jakub Kicinski wrote: > > Currently when CONFIG_TLS_DEVICE is set each time kTLS > > connection is opened and the offload is not successful > > (either because the underlying device doesn't support > > it or e.g. it's tables are full) a rate limited error > > will be printed to the logs. > > > > There is nothing wrong with failing TLS offload. SW > > path will process the packets just fine, avoid the > > noisy messages by demoting them from error to debug > > level. > > > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > Reviewed-by: Reviewed-by: Simon Horman <simon.horman@netronome.com> > > Did simon review this patch twice ;) ? > please fixup "Reviewed-by:" Tag. Ups :) > and also i think it shouldn't be even debug, it should be silent. > The device didn't advertise the feature, why would we print a debug > message on every TLS connection ? Coin toss :) I was wondering if the original author seen some value in those. I have some tls tracepoints queued, those should cover set.
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index 9f3bdbc1e593..9d67989ab7a7 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -865,8 +865,8 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx) } if (!(netdev->features & NETIF_F_HW_TLS_RX)) { - pr_err_ratelimited("%s: netdev %s with no TLS offload\n", - __func__, netdev->name); + pr_debug_ratelimited("%s: netdev %s with no TLS offload\n", + __func__, netdev->name); rc = -ENOTSUPP; goto release_netdev; } @@ -895,8 +895,8 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx) &ctx->crypto_recv.info, tcp_sk(sk)->copied_seq); if (rc) { - pr_err_ratelimited("%s: The netdev has refused to offload this socket\n", - __func__); + pr_debug_ratelimited("%s: The netdev has refused to offload this socket\n", + __func__); goto free_sw_resources; }