Re: [PATCH v5 17/18] pesto, conf: Send updated rules from pesto back to passt/pasta

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 29 Apr 2026 15:21:36 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Sat, Apr 25, 2026 at 11:36:05AM +0200, Stefano Brivio wrote:
> > On Fri, 24 Apr 2026 18:38:57 -0400
> > Jon Maloy <jmaloy@redhat.com> wrote:
> >   
> > > On 2026-04-21 02:25, David Gibson wrote:  
> > > > Extend pesto to send the updated rule configuration back to passt/pasta.
> > > > Extend passt/pasta to read the new configuration and store the new rules in
> > > > a "pending" table.   We don't yet attempt to activate them.
> > > > 
> > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > > > Message-ID: <20260322141843.4095972-3-sbrivio@redhat.com>
> > > > [dwg: Based on an early draft from Stefano]\
> > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>    
> > > 
> > > [...]  
> > > >   
> > > > +/**
> > > > + * conf_recv_rules() - Receive forwarding rules from configuration client
> > > > + * @c:		Execution context
> > > > + * @fd:		Socket to the client
> > > > + *
> > > > + * Return: 0 on success, -1 on failure
> > > > + */
> > > > +static int conf_recv_rules(const struct ctx *c, int fd)
> > > > +{
> > > > +	while (1) {
> > > > +		struct fwd_table *fwd;
> > > > +		struct fwd_rule r;
> > > > +		uint32_t count;
> > > > +		uint8_t pif;
> > > > +		unsigned i;
> > > > +
> > > > +		if (read_u8(fd, &pif))
> > > > +			return -1;
> > > > +
> > > > +		if (pif == PIF_NONE)
> > > > +			break;
> > > > +
> > > > +		if (pif >= ARRAY_SIZE(c->fwd_pending) ||
> > > > +		    !(fwd = c->fwd_pending[pif])) {
> > > > +			err("Received rules for non-existent table");
> > > > +			return -1;
> > > > +		}
> > > > +
> > > > +		if (read_u32(fd, &count))
> > > > +			return -1;
> > > > +
> > > > +		if (count > MAX_FWD_RULES) {
> > > > +			err("Received %"PRIu32" rules (maximum %u)",
> > > > +			    count, MAX_FWD_RULES);
> > > > +			return -1;
> > > > +		}
> > > > +
> > > > +		for (i = 0; i < count; i++) {
> > > > +			fwd_rule_read(fd, &r);    
> > > 
> > > Since we don't check the return value I think we risk passing an only 
> > > partially initialized fwd_rule to fwd_rule_add() if the read fails.
> > > Maybe:
> > >    if (fwd_rule_read(fd, &r))
> > >        return -1;  
> > 
> > Right, yes, that makes sense in general, even though I think this will
> > need a small rework (I didn't get to that yet) to implement this point
> > of the to-do list (see cover letter):
> >   
> > > - Don't allow a client which sends a partial configuration then
> > >   blocks also block passt  
> 
> Right.  In retrospect this requirement makes the way I structured the
> helpers in serialise.c not so helpful after all.
> 
> > ...because at that point we'll want to permit partial reads and keep a
> > buffer with a counter of received bytes (perhaps rules / PIFs too).
> > 
> > But actually it doesn't even need to be in this series or in a first
> > implementation. It could simply be a limitation (in that case, I'll add
> > the return -1 you suggest).
> > 
> > A user who can connect to passt could anyway configure it to be useless
> > so I don't see any particular security concern with it.  
> 
> That's a good point.  At the moment the limitations of the protocol
> (specifically the lack of TAP rules) limits the amount of damage a
> client can do, but we do hope to extend that, so I think the argument
> makes sense anyway.

Good, changed to include return -1 in v6.

-- 
Stefano