[PATCH v2] xen: update pvcalls_front_accept prototype

[PATCH v2] xen: update pvcalls_front_accept prototype

[Stefano Stabellini]

xen: update pvcalls_front_accept prototype

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---

Changes in v2:
- also update pvcalls-front.c

diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index b72ee9379d77..cab480059731 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -769,7 +769,8 @@ int pvcalls_front_listen(struct socket *sock, int backlog)
 	return ret;
 }
 
-int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags)
+int pvcalls_front_accept(struct socket *sock, struct socket *newsock,
+			 struct proto_accept_arg *arg)
 {
 	struct pvcalls_bedata *bedata;
 	struct sock_mapping *map;
@@ -788,7 +789,7 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags)
 		return -EINVAL;
 	}
 
-	nonblock = flags & SOCK_NONBLOCK;
+	nonblock = arg->flags & SOCK_NONBLOCK;
 	/*
 	 * Backend only supports 1 inflight accept request, will return
 	 * errors for the others
diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h
index f694ad77379f..881ef14660bc 100644
--- a/drivers/xen/pvcalls-front.h
+++ b/drivers/xen/pvcalls-front.h
@@ -12,7 +12,7 @@ int pvcalls_front_bind(struct socket *sock,
 int pvcalls_front_listen(struct socket *sock, int backlog);
 int pvcalls_front_accept(struct socket *sock,
 			 struct socket *newsock,
-			 int flags);
+			 struct proto_accept_arg *arg);
 int pvcalls_front_sendmsg(struct socket *sock,
 			  struct msghdr *msg,
 			  size_t len);

Re: [PATCH v2] xen: update pvcalls_front_accept prototype

[Jan Beulich]

On 06.01.2025 22:36, Stefano Stabellini wrote:
> xen: update pvcalls_front_accept prototype
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> 
> Changes in v2:
> - also update pvcalls-front.c

The patch still gives the impression of being incomplete: There's no
caller of the function that you update. However, there's no such caller
in the first place. Why don't you just delete the function then?

Jan

> diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
> index b72ee9379d77..cab480059731 100644
> --- a/drivers/xen/pvcalls-front.c
> +++ b/drivers/xen/pvcalls-front.c
> @@ -769,7 +769,8 @@ int pvcalls_front_listen(struct socket *sock, int backlog)
>  	return ret;
>  }
>  
> -int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags)
> +int pvcalls_front_accept(struct socket *sock, struct socket *newsock,
> +			 struct proto_accept_arg *arg)
>  {
>  	struct pvcalls_bedata *bedata;
>  	struct sock_mapping *map;
> @@ -788,7 +789,7 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags)
>  		return -EINVAL;
>  	}
>  
> -	nonblock = flags & SOCK_NONBLOCK;
> +	nonblock = arg->flags & SOCK_NONBLOCK;
>  	/*
>  	 * Backend only supports 1 inflight accept request, will return
>  	 * errors for the others
> diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h
> index f694ad77379f..881ef14660bc 100644
> --- a/drivers/xen/pvcalls-front.h
> +++ b/drivers/xen/pvcalls-front.h
> @@ -12,7 +12,7 @@ int pvcalls_front_bind(struct socket *sock,
>  int pvcalls_front_listen(struct socket *sock, int backlog);
>  int pvcalls_front_accept(struct socket *sock,
>  			 struct socket *newsock,
> -			 int flags);
> +			 struct proto_accept_arg *arg);
>  int pvcalls_front_sendmsg(struct socket *sock,
>  			  struct msghdr *msg,
>  			  size_t len);
> 

Re: [PATCH v2] xen: update pvcalls_front_accept prototype

[Stefano Stabellini]

On Tue, 7 Jan 2025, Jan Beulich wrote:
> On 06.01.2025 22:36, Stefano Stabellini wrote:
> > xen: update pvcalls_front_accept prototype
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > ---
> > 
> > Changes in v2:
> > - also update pvcalls-front.c
> 
> The patch still gives the impression of being incomplete: There's no
> caller of the function that you update. However, there's no such caller
> in the first place. Why don't you just delete the function then?

It is meant to be called from an out-of-tree module, which has not been
upstreamed yet


> > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
> > index b72ee9379d77..cab480059731 100644
> > --- a/drivers/xen/pvcalls-front.c
> > +++ b/drivers/xen/pvcalls-front.c
> > @@ -769,7 +769,8 @@ int pvcalls_front_listen(struct socket *sock, int backlog)
> >  	return ret;
> >  }
> >  
> > -int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags)
> > +int pvcalls_front_accept(struct socket *sock, struct socket *newsock,
> > +			 struct proto_accept_arg *arg)
> >  {
> >  	struct pvcalls_bedata *bedata;
> >  	struct sock_mapping *map;
> > @@ -788,7 +789,7 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags)
> >  		return -EINVAL;
> >  	}
> >  
> > -	nonblock = flags & SOCK_NONBLOCK;
> > +	nonblock = arg->flags & SOCK_NONBLOCK;
> >  	/*
> >  	 * Backend only supports 1 inflight accept request, will return
> >  	 * errors for the others
> > diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h
> > index f694ad77379f..881ef14660bc 100644
> > --- a/drivers/xen/pvcalls-front.h
> > +++ b/drivers/xen/pvcalls-front.h
> > @@ -12,7 +12,7 @@ int pvcalls_front_bind(struct socket *sock,
> >  int pvcalls_front_listen(struct socket *sock, int backlog);
> >  int pvcalls_front_accept(struct socket *sock,
> >  			 struct socket *newsock,
> > -			 int flags);
> > +			 struct proto_accept_arg *arg);
> >  int pvcalls_front_sendmsg(struct socket *sock,
> >  			  struct msghdr *msg,
> >  			  size_t len);
> > 
> 

Re: [PATCH v2] xen: update pvcalls_front_accept prototype

[Jan Beulich]

On 08.01.2025 00:30, Stefano Stabellini wrote:
> On Tue, 7 Jan 2025, Jan Beulich wrote:
>> On 06.01.2025 22:36, Stefano Stabellini wrote:
>>> xen: update pvcalls_front_accept prototype
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>> ---
>>>
>>> Changes in v2:
>>> - also update pvcalls-front.c
>>
>> The patch still gives the impression of being incomplete: There's no
>> caller of the function that you update. However, there's no such caller
>> in the first place. Why don't you just delete the function then?
> 
> It is meant to be called from an out-of-tree module, which has not been
> upstreamed yet

And which then would require an EXPORT_SYMBOL() anyway. In Xen, as you're
well aware, such unreachable code would actually constitute a Misra
violation.

Without any in-tree caller, imo the change needs a non-empty description,
clarifying why the adjustment is wanted / needed.

Jan

Re: [PATCH v2] xen: update pvcalls_front_accept prototype

[Stefano Stabellini]

On Wed, 8 Jan 2025, Jan Beulich wrote:
> On 08.01.2025 00:30, Stefano Stabellini wrote:
> > On Tue, 7 Jan 2025, Jan Beulich wrote:
> >> On 06.01.2025 22:36, Stefano Stabellini wrote:
> >>> xen: update pvcalls_front_accept prototype
> >>>
> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - also update pvcalls-front.c
> >>
> >> The patch still gives the impression of being incomplete: There's no
> >> caller of the function that you update. However, there's no such caller
> >> in the first place. Why don't you just delete the function then?
> > 
> > It is meant to be called from an out-of-tree module, which has not been
> > upstreamed yet
> 
> And which then would require an EXPORT_SYMBOL() anyway. In Xen, as you're
> well aware, such unreachable code would actually constitute a Misra
> violation.
> 
> Without any in-tree caller, imo the change needs a non-empty description,
> clarifying why the adjustment is wanted / needed.

How about:

---
xen: update pvcalls_front_accept prototype

While currently there are no in-tree callers of these functions, it is
best to keep them up-to-date with the latest network API.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Re: [PATCH v2] xen: update pvcalls_front_accept prototype

[Jan Beulich]

On 10.01.2025 01:10, Stefano Stabellini wrote:
> On Wed, 8 Jan 2025, Jan Beulich wrote:
>> On 08.01.2025 00:30, Stefano Stabellini wrote:
>>> On Tue, 7 Jan 2025, Jan Beulich wrote:
>>>> On 06.01.2025 22:36, Stefano Stabellini wrote:
>>>>> xen: update pvcalls_front_accept prototype
>>>>>
>>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - also update pvcalls-front.c
>>>>
>>>> The patch still gives the impression of being incomplete: There's no
>>>> caller of the function that you update. However, there's no such caller
>>>> in the first place. Why don't you just delete the function then?
>>>
>>> It is meant to be called from an out-of-tree module, which has not been
>>> upstreamed yet
>>
>> And which then would require an EXPORT_SYMBOL() anyway. In Xen, as you're
>> well aware, such unreachable code would actually constitute a Misra
>> violation.
>>
>> Without any in-tree caller, imo the change needs a non-empty description,
>> clarifying why the adjustment is wanted / needed.
> 
> How about:
> 
> ---
> xen: update pvcalls_front_accept prototype
> 
> While currently there are no in-tree callers of these functions, it is
> best to keep them up-to-date with the latest network API.

SGTM, fwiw.

Jan