Re: [PATCH v3 1/6] argo: lower level of noisy connection-refused log

[Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

From: Jason Andryuk <jason.andryuk@xxxxxxx>
To: "Daniel P. Smith"<dpsmith@xxxxxxxxxxxxxxxxxxxx>, <dmukhin@xxxxxxxx>, 
<xen-devel@xxxxxxxxxxxxxxxxxxxx>
Cc: <andrew.cooper3@xxxxxxxxxx>, <anthony.perard@xxxxxxxxxx>, 
<jbeulich@xxxxxxxx>, <julien@xxxxxxx>, <michal.orzel@xxxxxxx>, 
<roger.pau@xxxxxxxxxx>, <sstabellini@xxxxxxxxxx>, 
<christopher.w.clark@xxxxxxxxx>, "Mykola Kvach"<mykola_kvach@xxxxxxxx>
Date: Mon, 08 Jun 2026 19:16:01 -0400
Subject: Re: [PATCH v3 1/6] argo: lower level of noisy connection-refused log

 > On 2026-06-08 15:54, Daniel P. Smith wrote: 
 > > 
 > > 
 > > On 5/26/26 5:58 PM, dmukhin@xxxxxxxx wrote: 
 > >> From: Denis Mukhin <dmukhin@xxxxxxxx> 
 > >> 
 > >> Switch the log line to argo_dprintk() so it is enabled only in debug 
 > >> environments, as it can spam the logs when a dom0 service using the Argo 
 > >> hypercall tries to communicate with a domain that is still starting up. 
 > >> 
 > >> Note that this also lowers the log level to debug when the argo_dprintk() 
 > >> facility is enabled. 
 > >> 
 > >> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx> 
 > >> Reviewed-by: Mykola Kvach <mykola_kvach@xxxxxxxx> 
 > >> --- 
 > >> Changes since v2: 
 > >> - updated commit message 
 > >> --- 
 > >>   xen/common/argo.c | 7 +++---- 
 > >>   1 file changed, 3 insertions(+), 4 deletions(-) 
 > >> 
 > >> diff --git a/xen/common/argo.c b/xen/common/argo.c 
 > >> index 28626e00a8cb..98a3db7fd070 100644 
 > >> --- a/xen/common/argo.c 
 > >> +++ b/xen/common/argo.c 
 > >> @@ -2034,10 +2034,9 @@ sendv(struct domain *src_d, xen_argo_addr_t 
 > >> *src_addr, 
 > >>                                           src_id.domain_id); 
 > >>       if ( !ring_info ) 
 > >>       { 
 > >> -        gprintk(XENLOG_ERR, 
 > >> -                "argo: vm%u connection refused, src (vm%u:%x) dst 
 > >> (vm%u:%x)\n", 
 > >> -                current->domain->domain_id, src_id.domain_id, 
 > >> src_id.aport, 
 > >> -                dst_addr->domain_id, dst_addr->aport); 
 > >> +        argo_dprintk("vm%u connection refused, src (vm%u:%x) dst 
 > >> (vm%u:%x)\n", 
 > >> +                     current->domain->domain_id, src_id.domain_id, 
 > >> src_id.aport, 
 > >> +                     dst_addr->domain_id, dst_addr->aport); 
 > >>           ret = -ECONNREFUSED; 
 > >>       } 
 > > 
 > > My apologies but this is not the wisest approach, hitting this is a real 
 > > error and shouldn't be getting silenced. 
 >  
 > -ECONNREFUSED is still returned, and that is the important part, I think? 
 >  

Absolutely not. Argo at its essence is a security protocol where you want to 
minimize the amount of implicit trust we have to have with the endpoint. 
Telling a bad actor he did a bad action tells you nothing. The send operation 
is the critical security path and you must have an auditable record that an 
endpoint misbehaved. If yo want to implicitly trust your end point after 
passing the accees check, then you can just use grants. 

 > While gprintk(), it is trivially guest triggerable, so I think it wants 
 > to be a debug message like this change made it.  As a comparison, errors 
 > in event_channel.c are gdprintk(). 
 >  

Again, this message should never be disabled which could occur with gdprintk() 
depending on the NDEBUG flag. HMX and the Xen implementation Argo are high 
assurance mechanisms for high assureance implementations. The maintainers must 
ensure that foundational principles are not compromised. If an implementer 
chosing Argo, it is because of the properties that come about because of the 
design of this mechanism.

A side note, gprintk is rate limited, so an option is to adjust your rate limit.

 > If you are seeing a lot of 
 > > these messages, I would suggest asking yourself why. Without further 
 > > context on how you are using it, one suggesting is perhaps your 
 > > connection model might need to be revisited. 
 >  
 > There isn't a way to know if a port is available without polling? 


That is absolutely not a true statement. Send a notify message with a request 
for a chunk of data, ideally the amount you want to send. You will get a 
response that includes a flag to tell you the ring exists[1] and how much space 
is available [2]. If the ring doesn't exist, the response will have 0 in both 
flags and max_message_size. I would even suggest reviewing the flags, as a few 
of them might also be of interest. For instance, the ring empty flag. If the 
ring empty flag is set and max_message_size is smaller than the amount of data 
you are attempting to send, your attempt to send will fail. Whereas, you can 
use this on your sender size to packetize your data depending on what kind of 
transmit strategy you want, e.g. willing to allow a single send to max out the 
buffer.

I would add that you are free to flood the notify op in a poll loop and not see 
a flood to the message buffer since that path is audited with argo_dprintk's.

[1] https://elixir.bootlin.com/xen/v4.21.1/source/xen/common/argo.c#L1355
[2] https://elixir.bootlin.com/xen/v4.21.1/source/xen/common/argo.c#L1410


Finally, while looking at the series further and the example CI build due to 
the SNAFU, I would like to revise my review of the other patches. Since we are 
touching all these log points in this series, perhaps it would be good to 
update the fmt to new format specifiers such as %pd for domain identification, 
as well as reconsider whether logging the virutal address is the correct detail 
to be reported.

v/r,
dps