On Tue, May 05, 2026 at 09:53:04AM +0200, Laurent Vivier wrote: > On 5/5/26 01:11, Stefano Brivio wrote: > > From: David Gibson > > > > 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 > > [dwg: Based on an early draft from Stefano] > > [sbrivio: Add redundant check on interface names being terminated in > > conf_recv_rules(), to make static checkers happy] > > [sbrivio: Make conf_recv_rules() return -1 if fwd_rule_read() fails, > > as suggested by Jon Maloy] > > Signed-off-by: David Gibson > > Reviewed-by: Laurent Vivier > > But one comment below > > > --- > > Makefile | 5 --- > > conf.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-------- > > fwd.c | 10 +++++- > > passt.h | 2 ++ > > pesto.c | 35 +++++++++++++++++++++ > > 5 files changed, 127 insertions(+), 19 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index c746b55..ae755a0 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -224,10 +224,6 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck > > $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^ > > passt.cppcheck: BASE_CPPFLAGS += -UPESTO > > -passt.cppcheck: CPPCHECK_FLAGS += \ > > - --suppress=unusedFunction:fwd_rule.c \ > > - --suppress=staticFunction:fwd_rule.c \ > > - --suppress=unusedFunction:serialise.c > > passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h > > passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h > > @@ -238,7 +234,6 @@ pesto.cppcheck: CPPCHECK_FLAGS += \ > > --suppress=unusedFunction:inany.h \ > > --suppress=unusedFunction:inany.c \ > > --suppress=unusedFunction:ip.h \ > > - --suppress=unusedFunction:fwd_rule.c \ > > --suppress=staticFunction:fwd_rule.c \ > > --suppress=unusedFunction:serialise.c > > pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h > > diff --git a/conf.c b/conf.c > > index 5e4e81e..f035fd3 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -1971,6 +1971,62 @@ static int conf_send_rules(const struct ctx *c, int fd) > > return 0; > > } > > +/** > > + * 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++) { > > + if (fwd_rule_read(fd, &r)) > > + return -1; > > + > > + if (r.ifname[sizeof(r.ifname) - 1]) { > > + err("Interface name was not NULL terminated"); > > + return -1; > > + } > > + /* Redundant, to make static checkers happy */ > > + r.ifname[sizeof(r.ifname) - 1] = '\0'; > > + > > + if (fwd_rule_add(fwd, &r) < 0) > > + return -1; > > + } > > + } > > + > > + return 0; > > +} > > + > > /** > > * conf_close() - Close configuration / control socket and clean up > > * @c: Execution context > > @@ -2075,21 +2131,33 @@ fail: > > void conf_handler(struct ctx *c, uint32_t events) > > { > > if (events & EPOLLIN) { > > - char discard[BUFSIZ]; > > - ssize_t n; > > - > > - do { > > - n = read(c->fd_control, discard, sizeof(discard)); > > - if (n > 0) > > - debug("Discarded %zd bytes of config data", n); > > - } while (n > 0); > > - if (n == 0) { > > - debug("Configuration client EOF"); > > - goto close; > > + unsigned pif; > > + > > + /* Clear pending tables */ > > + for (pif = 0; pif < PIF_NUM_TYPES; pif++) { > > + struct fwd_table *fwd = c->fwd_pending[pif]; > > + > > + if (!fwd) > > + continue; > > + fwd->count = 0; > > + fwd->sock_count = 0; > > } > > - if (errno != EAGAIN && errno != EWOULDBLOCK) { > > - err_perror("Error reading config data"); > > + > > + /* FIXME: this could block indefinitely if the client doesn't > > + * write as much as it should > > + */ > > + if (conf_recv_rules(c, c->fd_control) < 0) > > goto close; > > + > > + for (pif = 0; pif < PIF_NUM_TYPES; pif++) { > > + struct fwd_table *fwd = c->fwd_pending[pif]; > > + > > + if (!fwd) > > + continue; > > + > > + info("New forwarding rules for %s:", pif_name(pif)); > > + fwd_rules_dump(info, fwd->rules, fwd->count, > > + " ", ""); > > } > > } > > diff --git a/fwd.c b/fwd.c > > index 8849cfc..d93d2e5 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -247,6 +247,9 @@ void fwd_neigh_table_init(const struct ctx *c) > > static struct fwd_table fwd_in; > > static struct fwd_table fwd_out; > > +static struct fwd_table fwd_in_pending; > > +static struct fwd_table fwd_out_pending; > > + > > /** > > * fwd_rule_init() - Initialise forwarding tables > > * @c: Execution context > > @@ -269,10 +272,15 @@ void fwd_rule_init(struct ctx *c) > > caps |= FWD_CAP_IFNAME; > > fwd_in.caps = fwd_out.caps = caps; > > + fwd_in_pending.caps = fwd_out_pending.caps = caps; > > c->fwd[PIF_HOST] = &fwd_in; > > - if (c->mode == MODE_PASTA) > > + c->fwd_pending[PIF_HOST] = &fwd_in_pending; > > + > > + if (c->mode == MODE_PASTA) { > > c->fwd[PIF_SPLICE] = &fwd_out; > > + c->fwd_pending[PIF_SPLICE] = &fwd_out_pending; > > + } > > } > > /** > > diff --git a/passt.h b/passt.h > > index b3f049d..1726965 100644 > > --- a/passt.h > > +++ b/passt.h > > @@ -188,6 +188,7 @@ struct ip6_ctx { > > * @pasta_ifi: Index of namespace interface for pasta > > * @pasta_conf_ns: Configure namespace after creating it > > * @fwd: Forwarding tables > > + * @fwd_pending: Pending forward tables > > * @no_tcp: Disable TCP operation > > * @tcp: Context for TCP protocol handler > > * @no_udp: Disable UDP operation > > @@ -270,6 +271,7 @@ struct ctx { > > int pasta_conf_ns; > > struct fwd_table *fwd[PIF_NUM_TYPES]; > > + struct fwd_table *fwd_pending[PIF_NUM_TYPES]; > > int no_tcp; > > struct tcp_ctx tcp; > > diff --git a/pesto.c b/pesto.c > > index 16b3a5a..73fdc39 100644 > > --- a/pesto.c > > +++ b/pesto.c > > @@ -230,6 +230,39 @@ static bool read_pif_conf(int fd, struct configuration *conf) > > return true; > > } > > +/** > > + * send_conf() - Send updated configuration to passt/pasta > > + * @fd: Control socket > > + * @conf: Updated configuration > > + */ > > +static void send_conf(int fd, const struct configuration *conf) > > +{ > > + unsigned i; > > + > > Perhaps it could be interesting to send a magic number (or a type id) if we > want to be able to update something else than the rules in the future? > We also can send the length of the data if we want to be able to ignore it > if the type id is not supported? > (Something like the chunks in IFF or PNG file format... but perhaps it's > overcomplicated for our purpose...) Undoubtedly, amongst a bunch of other ways we could make the protocol more flexible. However, I'm inclined to leave that v2. > > > + for (i = 0; i < conf->npifs; i++) { > > + const struct pif_configuration *pc = &conf->pif[i]; > > + unsigned j; > > + > > + if (write_u8(fd, pc->pif) < 0) > > + goto fail; > > + > > + if (write_u32(fd, pc->fwd.count) < 0) > > + goto fail; > > + > > + for (j = 0; j < pc->fwd.count; j++) { > > + if (fwd_rule_write(fd, &pc->fwd.rules[j]) < 0) > > + goto fail; > > + } > > + } > > + > > + if (write_u8(fd, PIF_NONE) < 0) > > + goto fail; > > + return; > > + > > +fail: > > + die_perror("Error writing to control socket"); > > +} > > + > > /** > > * show_conf() - Show current configuration obtained from passt/pasta > > * @conf: Configuration description > > @@ -432,6 +465,8 @@ int main(int argc, char **argv) > > show_conf(&conf); > > } > > + send_conf(s, &conf); > > + > > noupdate: > > if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0) > > die_perror("Error shutting down control socket"); > -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson