| Message ID | 20230609125153.3919-2-hare@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/tls: fixes for NVMe-over-TLS | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Hi Hannes, 2023-06-09, 14:51:50 +0200, Hannes Reinecke wrote: > tls_sw_sendmsg() / tls_do_sw_sendpage() already handles > MSG_MORE / MSG_SENDPAGE_NOTLAST, but bails out on MSG_EOR. > But seeing that MSG_EOR is basically the opposite of > MSG_MORE / MSG_SENDPAGE_NOTLAST this patch adds handling > MSG_EOR by treating it as the negation of MSG_MORE. > > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: netdev@vger.kernel.org > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > net/tls/tls_sw.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c > index 635b8bf6b937..be8e0459d403 100644 > --- a/net/tls/tls_sw.c > +++ b/net/tls/tls_sw.c > @@ -953,9 +953,12 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) > int pending; > > if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | > - MSG_CMSG_COMPAT)) > + MSG_EOR | MSG_CMSG_COMPAT)) > return -EOPNOTSUPP; > > + if (msg->msg_flags & MSG_EOR) > + eor = true; Is MSG_EOR supposed to be incompatible with MSG_MORE, or is it supposed to cancel it? (ie: MSG_MORE | MSG_EOR is invalid, or MSG_MORE | MSG_EOR behaves like MSG_EOR) The current code already behaves as if _EOR was passed as long as MSG_MORE isn't passed, so _EOR is only needed to cancel out _MORE (or in your case, because NVMe-over-TLS sets it). If _EOR and _MORE (or MSG_SENDPAGE_NOTLAST below) are supposed to be incompatible, we should return an error when they're both set. If we accept both flags being set at the same time, I think we should document the expected behavior ("_EOR overrides _MORE/_NOTLAST") and add specific selftests to avoid regressions.
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 635b8bf6b937..be8e0459d403 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -953,9 +953,12 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) int pending; if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | - MSG_CMSG_COMPAT)) + MSG_EOR | MSG_CMSG_COMPAT)) return -EOPNOTSUPP; + if (msg->msg_flags & MSG_EOR) + eor = true; + ret = mutex_lock_interruptible(&tls_ctx->tx_lock); if (ret) return ret; @@ -1173,6 +1176,8 @@ static int tls_sw_do_sendpage(struct sock *sk, struct page *page, bool eor; eor = !(flags & MSG_SENDPAGE_NOTLAST); + if (flags & MSG_EOR) + eor = true; sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk); /* Call the sk_stream functions to manage the sndbuf mem. */ @@ -1274,7 +1279,7 @@ static int tls_sw_do_sendpage(struct sock *sk, struct page *page, int tls_sw_sendpage_locked(struct sock *sk, struct page *page, int offset, size_t size, int flags) { - if (flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | + if (flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | MSG_EOR | MSG_SENDPAGE_NOTLAST | MSG_SENDPAGE_NOPOLICY | MSG_NO_SHARED_FRAGS)) return -EOPNOTSUPP; @@ -1288,7 +1293,7 @@ int tls_sw_sendpage(struct sock *sk, struct page *page, struct tls_context *tls_ctx = tls_get_ctx(sk); int ret; - if (flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | + if (flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | MSG_EOR | MSG_SENDPAGE_NOTLAST | MSG_SENDPAGE_NOPOLICY)) return -EOPNOTSUPP;
tls_sw_sendmsg() / tls_do_sw_sendpage() already handles MSG_MORE / MSG_SENDPAGE_NOTLAST, but bails out on MSG_EOR. But seeing that MSG_EOR is basically the opposite of MSG_MORE / MSG_SENDPAGE_NOTLAST this patch adds handling MSG_EOR by treating it as the negation of MSG_MORE. Cc: Jakub Kicinski <kuba@kernel.org> Cc: netdev@vger.kernel.org Signed-off-by: Hannes Reinecke <hare@suse.de> --- net/tls/tls_sw.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)