Message ID | 20231214091947.395892-4-avkrasnov@salutedevices.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | send credit update during setting SO_RCVLOWAT | expand |
On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote: > Do not return if transport callback for SO_RCVLOWAT is set (only in > error case). In this case we don't need to set 'sk_rcvlowat' field in > each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat' > is now set only in af_vsock.c, change callback name from 'set_rcvlowat' > to 'notify_set_rcvlowat'. > > Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > Acked-by: Michael S. Tsirkin <mst@redhat.com> Maybe squash this with patch 2/4? > --- > Changelog: > v3 -> v4: > * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'. > * Commit message updated. > > include/net/af_vsock.h | 2 +- > net/vmw_vsock/af_vsock.c | 9 +++++++-- > net/vmw_vsock/hyperv_transport.c | 4 ++-- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h > index e302c0e804d0..535701efc1e5 100644 > --- a/include/net/af_vsock.h > +++ b/include/net/af_vsock.h > @@ -137,7 +137,6 @@ struct vsock_transport { > u64 (*stream_rcvhiwat)(struct vsock_sock *); > bool (*stream_is_active)(struct vsock_sock *); > bool (*stream_allow)(u32 cid, u32 port); > - int (*set_rcvlowat)(struct vsock_sock *vsk, int val); > > /* SEQ_PACKET. */ > ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, > @@ -168,6 +167,7 @@ struct vsock_transport { > struct vsock_transport_send_notify_data *); > /* sk_lock held by the caller */ > void (*notify_buffer_size)(struct vsock_sock *, u64 *); > + int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val); > > /* Shutdown. */ > int (*shutdown)(struct vsock_sock *, int); > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c > index 816725af281f..54ba7316f808 100644 > --- a/net/vmw_vsock/af_vsock.c > +++ b/net/vmw_vsock/af_vsock.c > @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val) > > transport = vsk->transport; > > - if (transport && transport->set_rcvlowat) > - return transport->set_rcvlowat(vsk, val); > + if (transport && transport->notify_set_rcvlowat) { > + int err; > + > + err = transport->notify_set_rcvlowat(vsk, val); > + if (err) > + return err; > + } > > WRITE_ONCE(sk->sk_rcvlowat, val ? : 1); > return 0; I would s > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c > index 7cb1a9d2cdb4..e2157e387217 100644 > --- a/net/vmw_vsock/hyperv_transport.c > +++ b/net/vmw_vsock/hyperv_transport.c > @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written, > } > > static > -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val) > +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val) > { > return -EOPNOTSUPP; > } > @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = { > .notify_send_pre_enqueue = hvs_notify_send_pre_enqueue, > .notify_send_post_enqueue = hvs_notify_send_post_enqueue, > > - .set_rcvlowat = hvs_set_rcvlowat > + .notify_set_rcvlowat = hvs_notify_set_rcvlowat > }; > > static bool hvs_check_transport(struct vsock_sock *vsk) > -- > 2.25.1
On 14.12.2023 13:29, Michael S. Tsirkin wrote: > On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote: >> Do not return if transport callback for SO_RCVLOWAT is set (only in >> error case). In this case we don't need to set 'sk_rcvlowat' field in >> each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat' >> is now set only in af_vsock.c, change callback name from 'set_rcvlowat' >> to 'notify_set_rcvlowat'. >> >> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com> >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> >> Acked-by: Michael S. Tsirkin <mst@redhat.com> > > Maybe squash this with patch 2/4? You mean just do 'git squash' without updating commit message manually? Thanks, Arseniy > >> --- >> Changelog: >> v3 -> v4: >> * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'. >> * Commit message updated. >> >> include/net/af_vsock.h | 2 +- >> net/vmw_vsock/af_vsock.c | 9 +++++++-- >> net/vmw_vsock/hyperv_transport.c | 4 ++-- >> 3 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h >> index e302c0e804d0..535701efc1e5 100644 >> --- a/include/net/af_vsock.h >> +++ b/include/net/af_vsock.h >> @@ -137,7 +137,6 @@ struct vsock_transport { >> u64 (*stream_rcvhiwat)(struct vsock_sock *); >> bool (*stream_is_active)(struct vsock_sock *); >> bool (*stream_allow)(u32 cid, u32 port); >> - int (*set_rcvlowat)(struct vsock_sock *vsk, int val); >> >> /* SEQ_PACKET. */ >> ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, >> @@ -168,6 +167,7 @@ struct vsock_transport { >> struct vsock_transport_send_notify_data *); >> /* sk_lock held by the caller */ >> void (*notify_buffer_size)(struct vsock_sock *, u64 *); >> + int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val); >> >> /* Shutdown. */ >> int (*shutdown)(struct vsock_sock *, int); >> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >> index 816725af281f..54ba7316f808 100644 >> --- a/net/vmw_vsock/af_vsock.c >> +++ b/net/vmw_vsock/af_vsock.c >> @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val) >> >> transport = vsk->transport; >> >> - if (transport && transport->set_rcvlowat) >> - return transport->set_rcvlowat(vsk, val); >> + if (transport && transport->notify_set_rcvlowat) { >> + int err; >> + >> + err = transport->notify_set_rcvlowat(vsk, val); >> + if (err) >> + return err; >> + } >> >> WRITE_ONCE(sk->sk_rcvlowat, val ? : 1); >> return 0; > > > > I would s > >> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c >> index 7cb1a9d2cdb4..e2157e387217 100644 >> --- a/net/vmw_vsock/hyperv_transport.c >> +++ b/net/vmw_vsock/hyperv_transport.c >> @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written, >> } >> >> static >> -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val) >> +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val) >> { >> return -EOPNOTSUPP; >> } >> @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = { >> .notify_send_pre_enqueue = hvs_notify_send_pre_enqueue, >> .notify_send_post_enqueue = hvs_notify_send_post_enqueue, >> >> - .set_rcvlowat = hvs_set_rcvlowat >> + .notify_set_rcvlowat = hvs_notify_set_rcvlowat >> }; >> >> static bool hvs_check_transport(struct vsock_sock *vsk) >> -- >> 2.25.1 >
On Thu, Dec 14, 2023 at 01:52:50PM +0300, Arseniy Krasnov wrote: > > > On 14.12.2023 13:29, Michael S. Tsirkin wrote: > > On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote: > >> Do not return if transport callback for SO_RCVLOWAT is set (only in > >> error case). In this case we don't need to set 'sk_rcvlowat' field in > >> each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat' > >> is now set only in af_vsock.c, change callback name from 'set_rcvlowat' > >> to 'notify_set_rcvlowat'. > >> > >> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com> > >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > >> Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > > Maybe squash this with patch 2/4? > > You mean just do 'git squash' without updating commit message manually? > > Thanks, Arseniy commit message should reflect all that patch does. > > > >> --- > >> Changelog: > >> v3 -> v4: > >> * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'. > >> * Commit message updated. > >> > >> include/net/af_vsock.h | 2 +- > >> net/vmw_vsock/af_vsock.c | 9 +++++++-- > >> net/vmw_vsock/hyperv_transport.c | 4 ++-- > >> 3 files changed, 10 insertions(+), 5 deletions(-) > >> > >> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h > >> index e302c0e804d0..535701efc1e5 100644 > >> --- a/include/net/af_vsock.h > >> +++ b/include/net/af_vsock.h > >> @@ -137,7 +137,6 @@ struct vsock_transport { > >> u64 (*stream_rcvhiwat)(struct vsock_sock *); > >> bool (*stream_is_active)(struct vsock_sock *); > >> bool (*stream_allow)(u32 cid, u32 port); > >> - int (*set_rcvlowat)(struct vsock_sock *vsk, int val); > >> > >> /* SEQ_PACKET. */ > >> ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, > >> @@ -168,6 +167,7 @@ struct vsock_transport { > >> struct vsock_transport_send_notify_data *); > >> /* sk_lock held by the caller */ > >> void (*notify_buffer_size)(struct vsock_sock *, u64 *); > >> + int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val); > >> > >> /* Shutdown. */ > >> int (*shutdown)(struct vsock_sock *, int); > >> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c > >> index 816725af281f..54ba7316f808 100644 > >> --- a/net/vmw_vsock/af_vsock.c > >> +++ b/net/vmw_vsock/af_vsock.c > >> @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val) > >> > >> transport = vsk->transport; > >> > >> - if (transport && transport->set_rcvlowat) > >> - return transport->set_rcvlowat(vsk, val); > >> + if (transport && transport->notify_set_rcvlowat) { > >> + int err; > >> + > >> + err = transport->notify_set_rcvlowat(vsk, val); > >> + if (err) > >> + return err; > >> + } > >> > >> WRITE_ONCE(sk->sk_rcvlowat, val ? : 1); > >> return 0; > > > > > > > > I would s > > > >> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c > >> index 7cb1a9d2cdb4..e2157e387217 100644 > >> --- a/net/vmw_vsock/hyperv_transport.c > >> +++ b/net/vmw_vsock/hyperv_transport.c > >> @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written, > >> } > >> > >> static > >> -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val) > >> +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val) > >> { > >> return -EOPNOTSUPP; > >> } > >> @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = { > >> .notify_send_pre_enqueue = hvs_notify_send_pre_enqueue, > >> .notify_send_post_enqueue = hvs_notify_send_post_enqueue, > >> > >> - .set_rcvlowat = hvs_set_rcvlowat > >> + .notify_set_rcvlowat = hvs_notify_set_rcvlowat > >> }; > >> > >> static bool hvs_check_transport(struct vsock_sock *vsk) > >> -- > >> 2.25.1 > >
On 14.12.2023 13:29, Michael S. Tsirkin wrote: > On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote: >> Do not return if transport callback for SO_RCVLOWAT is set (only in >> error case). In this case we don't need to set 'sk_rcvlowat' field in >> each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat' >> is now set only in af_vsock.c, change callback name from 'set_rcvlowat' >> to 'notify_set_rcvlowat'. >> >> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com> >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> >> Acked-by: Michael S. Tsirkin <mst@redhat.com> > > Maybe squash this with patch 2/4? Done in v10 Thanks, Arseniy > >> --- >> Changelog: >> v3 -> v4: >> * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'. >> * Commit message updated. >> >> include/net/af_vsock.h | 2 +- >> net/vmw_vsock/af_vsock.c | 9 +++++++-- >> net/vmw_vsock/hyperv_transport.c | 4 ++-- >> 3 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h >> index e302c0e804d0..535701efc1e5 100644 >> --- a/include/net/af_vsock.h >> +++ b/include/net/af_vsock.h >> @@ -137,7 +137,6 @@ struct vsock_transport { >> u64 (*stream_rcvhiwat)(struct vsock_sock *); >> bool (*stream_is_active)(struct vsock_sock *); >> bool (*stream_allow)(u32 cid, u32 port); >> - int (*set_rcvlowat)(struct vsock_sock *vsk, int val); >> >> /* SEQ_PACKET. */ >> ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, >> @@ -168,6 +167,7 @@ struct vsock_transport { >> struct vsock_transport_send_notify_data *); >> /* sk_lock held by the caller */ >> void (*notify_buffer_size)(struct vsock_sock *, u64 *); >> + int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val); >> >> /* Shutdown. */ >> int (*shutdown)(struct vsock_sock *, int); >> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >> index 816725af281f..54ba7316f808 100644 >> --- a/net/vmw_vsock/af_vsock.c >> +++ b/net/vmw_vsock/af_vsock.c >> @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val) >> >> transport = vsk->transport; >> >> - if (transport && transport->set_rcvlowat) >> - return transport->set_rcvlowat(vsk, val); >> + if (transport && transport->notify_set_rcvlowat) { >> + int err; >> + >> + err = transport->notify_set_rcvlowat(vsk, val); >> + if (err) >> + return err; >> + } >> >> WRITE_ONCE(sk->sk_rcvlowat, val ? : 1); >> return 0; > > > > I would s > >> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c >> index 7cb1a9d2cdb4..e2157e387217 100644 >> --- a/net/vmw_vsock/hyperv_transport.c >> +++ b/net/vmw_vsock/hyperv_transport.c >> @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written, >> } >> >> static >> -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val) >> +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val) >> { >> return -EOPNOTSUPP; >> } >> @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = { >> .notify_send_pre_enqueue = hvs_notify_send_pre_enqueue, >> .notify_send_post_enqueue = hvs_notify_send_post_enqueue, >> >> - .set_rcvlowat = hvs_set_rcvlowat >> + .notify_set_rcvlowat = hvs_notify_set_rcvlowat >> }; >> >> static bool hvs_check_transport(struct vsock_sock *vsk) >> -- >> 2.25.1 >
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index e302c0e804d0..535701efc1e5 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h @@ -137,7 +137,6 @@ struct vsock_transport { u64 (*stream_rcvhiwat)(struct vsock_sock *); bool (*stream_is_active)(struct vsock_sock *); bool (*stream_allow)(u32 cid, u32 port); - int (*set_rcvlowat)(struct vsock_sock *vsk, int val); /* SEQ_PACKET. */ ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, @@ -168,6 +167,7 @@ struct vsock_transport { struct vsock_transport_send_notify_data *); /* sk_lock held by the caller */ void (*notify_buffer_size)(struct vsock_sock *, u64 *); + int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val); /* Shutdown. */ int (*shutdown)(struct vsock_sock *, int); diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 816725af281f..54ba7316f808 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val) transport = vsk->transport; - if (transport && transport->set_rcvlowat) - return transport->set_rcvlowat(vsk, val); + if (transport && transport->notify_set_rcvlowat) { + int err; + + err = transport->notify_set_rcvlowat(vsk, val); + if (err) + return err; + } WRITE_ONCE(sk->sk_rcvlowat, val ? : 1); return 0; diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c index 7cb1a9d2cdb4..e2157e387217 100644 --- a/net/vmw_vsock/hyperv_transport.c +++ b/net/vmw_vsock/hyperv_transport.c @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written, } static -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val) +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val) { return -EOPNOTSUPP; } @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = { .notify_send_pre_enqueue = hvs_notify_send_pre_enqueue, .notify_send_post_enqueue = hvs_notify_send_post_enqueue, - .set_rcvlowat = hvs_set_rcvlowat + .notify_set_rcvlowat = hvs_notify_set_rcvlowat }; static bool hvs_check_transport(struct vsock_sock *vsk)