On Wed, Oct 11, 2023 at 11:55:03AM +0200, Pablo Neira Ayuso wrote: > You have to set an initial label if you plan to update it later on. If > conntrack comes with no initial label, then it is not possible to attach > it later because conntrack extensions are created by the time the new > entry is created. > > Skip entries with no label to skip ENOSPC error for conntracks that have > no initial label (this is assuming a scenario with conntracks with and > _without_ labels is possible, and the conntrack command line tool is used > to update all entries regardless they have or not an initial label, e.g. > conntrack -U --label-add "testlabel". Still not fully correct. Current behaviour is: If there is at least one rule in the ruleset that uses the connlabel, then connlabel conntrack extension is always allocated. I wonder if this needs a sysctl toggle just like nf_conntrack_timestamp. Otherwise I am not sure how to document this. > # conntrack -U --label-add testlabel --dst 9.9.9.9 > icmp 1 13 src=192.168.2.130 dst=9.9.9.9 type=8 code=0 id=50997 src=9.9.9.9 dst=192.168.2.130 type=0 code=0 id=50997 mark=0 use=2 labels=default,testlabel > conntrack v1.4.8 (conntrack-tools): 1 flow entries have been updated. > # conntrack -C > 8 > > Note the remaining 7 conntracks have no label, hence, they could not be > updated. > > Update manpage to document this behaviour. > > Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1622 > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> > --- > conntrack.8 | 2 ++ > src/conntrack.c | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/conntrack.8 b/conntrack.8 > index 031eaa4e9fef..97c60079889f 100644 > --- a/conntrack.8 > +++ b/conntrack.8 > @@ -193,6 +193,8 @@ Use multiple \-l options to specify multiple labels that need to be set. > Specify the conntrack label to add to the selected conntracks. > This option is only available in conjunction with "\-I, \-\-create", > "\-A, \-\-add" or "\-U, \-\-update". > +You must set a default label for conntracks initially if you plan to update it > +later. "\-U, \-\-update" on conntracks with no initial entry will be ignored. > .TP > .BI "--label-del " "[LABEL]" > Specify the conntrack label to delete from the selected conntracks. > diff --git a/src/conntrack.c b/src/conntrack.c > index f9758d78d39b..06c2fee7ac4b 100644 > --- a/src/conntrack.c > +++ b/src/conntrack.c > @@ -2195,6 +2195,10 @@ static int mnl_nfct_update_cb(const struct nlmsghdr *nlh, void *data) > /* the entry has vanish in middle of the update */ > if (errno == ENOENT) > goto destroy_ok; > + else if (!(cmd->options & (CT_OPT_ADD_LABEL | CT_OPT_DEL_LABEL)) && > + errno == ENOSPC) This check is also not correct, this needs a v3. I have to check is ATTRS_CONNLABEL is set and cmd->options & (CT_OPT_ADD_LABEL | CT_OPT_DEL_LABEL) too, then check for ENOSPC, to avoid for bogus error reports to userspace. > + goto destroy_ok; > + > exit_error(OTHER_PROBLEM, > "Operation failed: %s", > err2str(errno, CT_UPDATE)); > -- > 2.30.2 >
