rmind/npf

Feature Request: Add `npfctl` frontend subcommand to `npf_table_replace()`

yazshel opened this issue · 2 comments

Hi again Mindaugas,

Thanks for accepting my previous PR :) As I suggested in the comments there, I was also
thinking of adding a command to npfctl to replace an active table with one rebuilt from a file.

I've actually got this pretty much working so will be submitting a PR for it shortly...

From the original comment on PR #38:

I was also thinking of adding a command to npfctl to replace an active table with one rebuilt from a file. Not only would this be handy for testing, it's also likely to have some real-world usefulness.
I wasn't 100% sure as to where to put this sub-command; keeping it with other commands under npfctl table makes the most sense from a usability perspective. But this does raise the issue of how to incorporate it within the current npfctl table subcommands: these currently all share the same ioctl(2), with a payload of npf_ioctl_table_t, and thus fit well together in the logic of the npfctl_table() command handler. However, adding a replace subcommand would involve a bit of messing up that neat subcommand logic, as the new IOC_NPF_TABLE_REPLACE ioctl is its own beast, more similar to load than anything else. So I'd need to restructure npfctl_table() a bit to accomodate this replace subcommand. Are you happy for me to give that a go, or would you prefer a different approach?

What I'm proposing would be to:

Add replace subcommand to npfctl_table() in src/npfctl/npfctl.c. A command test early in npfctl_table() would pass control to a new npfctl_table_replace() command handler function:
Add a new npfctl_table_replace(int fd, int argc, char **argv) function, which will:
a. Parse table name, type and filename from command line arguments
b. Check that table name exists in the active config and fetch its tid
c. Build a new nl_table_t with the passed name, type & file using a modified version of npfctl_build_table() (ie. split into 2 to decouple it from the nl_config_t npf_conf global variable, and also allow manually setting the TID)
d. Call npf_table_replace() with the new table structure.
Does that sound like an acceptable approach?

Cheers,

Timshel

rmind commented

Merged

How do we create this table ?
Can anyone help please.