On Mon, Feb 08, 2021 at 10:47:59AM -0800, Jakub Kicinski wrote:
> On Mon, 8 Feb 2021 10:41:35 -0800 Cong Wang wrote:
> > On Sat, Feb 6, 2021 at 9:26 PM <we...@ucloud.cn> wrote:
> > > + if (state && !(state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED)) {
> > > + NL_SET_ERR_MSG_ATTR(extack, tb,
> > > + "ct_state no trk, no other flag are
> > > set");
This one was imported from OvS but it's not accurate.
Should be more like: no trk, so no other flag can be set
or something like that.
Seems it doesn't need to explicitly mention "ct_state" in the msg,
btw. I can't check it right now but all other uses of
NL_SET_ERR_MSG_ATTR are not doing it, at least in cls_flower.c.
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (state & TCA_FLOWER_KEY_CT_FLAGS_NEW &&
> > > + state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) {
> > > + NL_SET_ERR_MSG_ATTR(extack, tb,
> > > + "ct_state new and est are
> > > exclusive");
> >
> > Please spell out the full words, "trk" and "est" are not good abbreviations.
>
> It does match user space naming in OvS as well as iproute2:
I also think it makes sense as is.
>
> { "trk", TCA_FLOWER_KEY_CT_FLAGS_TRACKED },
> { "new", TCA_FLOWER_KEY_CT_FLAGS_NEW },
> { "est", TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED },
> { "inv", TCA_FLOWER_KEY_CT_FLAGS_INVALID },
> { "rpl", TCA_FLOWER_KEY_CT_FLAGS_REPLY },
>
> IDK about netfilter itself.
>
