| Message ID | 20180604095818.7122-2-ecklm94@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [RFC,nft] parser: Set base chain prios with textual names | expand |
On 4 June 2018 at 11:58, Máté Eckl <ecklm94@gmail.com> wrote: > What I'm not sure of is: > - Are these token values considered user-friendly or usable? > - Is printing of these values with their names desired? > > What do you think? > > -- 8< -- > This patch adds the possibility to use textual names to set the chain priority > to basic values so that numeric values do not need to be learnt any more for > basic usage. > > Example: > nft> add table inet x > nft> add chain inet x y {type filter hook prerouting priority PRIO_MANGLE ;} > nft> list ruleset > table inet x { > chain y { > type filter hook prerouting priority -150; policy accept; > } > } > I believe the idea is good. But also, you should print the friendly names instead of the magic numbers :-P -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 04, 2018 at 11:58:18AM +0200, Máté Eckl wrote: > What I'm not sure of is: > - Are these token values considered user-friendly or usable? > - Is printing of these values with their names desired? > > What do you think? > > -- 8< -- > This patch adds the possibility to use textual names to set the chain priority > to basic values so that numeric values do not need to be learnt any more for > basic usage. > > Example: > nft> add table inet x > nft> add chain inet x y {type filter hook prerouting priority PRIO_MANGLE ;} that's fine, but I prefer more comprehensible (less programmer oriented) tag names, and also only expose the bare minimum that can be useful to start with, ie. those that are used by iptables chain definition included in tables. More comments below. > nft> list ruleset > table inet x { > chain y { > type filter hook prerouting priority -150; policy accept; > } > } > > Signed-off-by: Máté Eckl <ecklm94@gmail.com> > --- > src/parser_bison.y | 30 ++++++++++++++++++++++++++++-- > src/scanner.l | 13 +++++++++++++ > 2 files changed, 41 insertions(+), 2 deletions(-) > > diff --git a/src/parser_bison.y b/src/parser_bison.y > index 034dd01..236e9be 100644 > --- a/src/parser_bison.y > +++ b/src/parser_bison.y > @@ -21,6 +21,7 @@ > #include <linux/netfilter/nf_conntrack_tuple_common.h> > #include <linux/netfilter/nf_nat.h> > #include <linux/netfilter/nf_log.h> > +#include <linux/netfilter_ipv4.h> > #include <netinet/ip_icmp.h> > #include <netinet/icmp6.h> > #include <libnftnl/common.h> > @@ -313,6 +314,19 @@ int nft_lex(void *, void *, void *); > %token NEXTHDR "nexthdr" > %token HOPLIMIT "hoplimit" > > +%token PRIO_RAW_BEFORE_DEFRAG "PRIO_RAW_BEFORE_DEFRAG" > +%token PRIO_CONNTRACK_DEFRAG "PRIO_CONNTRACK_DEFRAG" > +%token PRIO_RAW "PRIO_RAW" > +%token PRIO_SELINUX_FIRST "PRIO_SELINUX_FIRST" > +%token PRIO_CONNTRACK "PRIO_CONNTRACK" > +%token PRIO_MANGLE "PRIO_MANGLE" > +%token PRIO_NAT_DST "PRIO_NAT_DST" > +%token PRIO_FILTER "PRIO_FILTER" > +%token PRIO_SECURITY "PRIO_SECURITY" > +%token PRIO_NAT_SRC "PRIO_NAT_SRC" > +%token PRIO_SELINUX_LAST "PRIO_SELINUX_LAST" > +%token PRIO_CONNTRACK_HELPER "PRIO_CONNTRACK_HELPER" We can probably handle this as strings, so we don't need to update scanner.l -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 04, 2018 at 12:10:49PM +0200, Arturo Borrero Gonzalez wrote: > On 4 June 2018 at 11:58, Máté Eckl <ecklm94@gmail.com> wrote: > > What I'm not sure of is: > > - Are these token values considered user-friendly or usable? > > - Is printing of these values with their names desired? > > > > What do you think? > > > > -- 8< -- > > This patch adds the possibility to use textual names to set the chain priority > > to basic values so that numeric values do not need to be learnt any more for > > basic usage. > > > > Example: > > nft> add table inet x > > nft> add chain inet x y {type filter hook prerouting priority PRIO_MANGLE ;} > > nft> list ruleset > > table inet x { > > chain y { > > type filter hook prerouting priority -150; policy accept; > > } > > } > > > > I believe the idea is good. But also, you should print the friendly > names instead of the magic numbers :-P Right, symmetry is desired thing in this. it would be cool if we could print based on approximate matching, eg. -150 would be printed as "mangle". but: -149 would be printed as "mangle + 1" so people could do arithmetics based on the tags. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 04, 2018 at 01:28:47PM +0200, Pablo Neira Ayuso wrote: > On Mon, Jun 04, 2018 at 11:58:18AM +0200, Máté Eckl wrote: > > What I'm not sure of is: > > - Are these token values considered user-friendly or usable? > > - Is printing of these values with their names desired? > > > > What do you think? > > > > -- 8< -- > > This patch adds the possibility to use textual names to set the chain priority > > to basic values so that numeric values do not need to be learnt any more for > > basic usage. > > > > Example: > > nft> add table inet x > > nft> add chain inet x y {type filter hook prerouting priority PRIO_MANGLE ;} > > that's fine, but I prefer more comprehensible (less programmer > oriented) tag names, and also only expose the bare minimum that can be > useful to start with, ie. those that are used by iptables chain > definition included in tables. Good idea, I will look after it. > > More comments below. > > > nft> list ruleset > > table inet x { > > chain y { > > type filter hook prerouting priority -150; policy accept; > > } > > } > > > > Signed-off-by: Máté Eckl <ecklm94@gmail.com> > > --- > > src/parser_bison.y | 30 ++++++++++++++++++++++++++++-- > > src/scanner.l | 13 +++++++++++++ > > 2 files changed, 41 insertions(+), 2 deletions(-) > > > > diff --git a/src/parser_bison.y b/src/parser_bison.y > > index 034dd01..236e9be 100644 > > --- a/src/parser_bison.y > > +++ b/src/parser_bison.y > > @@ -21,6 +21,7 @@ > > #include <linux/netfilter/nf_conntrack_tuple_common.h> > > #include <linux/netfilter/nf_nat.h> > > #include <linux/netfilter/nf_log.h> > > +#include <linux/netfilter_ipv4.h> > > #include <netinet/ip_icmp.h> > > #include <netinet/icmp6.h> > > #include <libnftnl/common.h> > > @@ -313,6 +314,19 @@ int nft_lex(void *, void *, void *); > > %token NEXTHDR "nexthdr" > > %token HOPLIMIT "hoplimit" > > > > +%token PRIO_RAW_BEFORE_DEFRAG "PRIO_RAW_BEFORE_DEFRAG" > > +%token PRIO_CONNTRACK_DEFRAG "PRIO_CONNTRACK_DEFRAG" > > +%token PRIO_RAW "PRIO_RAW" > > +%token PRIO_SELINUX_FIRST "PRIO_SELINUX_FIRST" > > +%token PRIO_CONNTRACK "PRIO_CONNTRACK" > > +%token PRIO_MANGLE "PRIO_MANGLE" > > +%token PRIO_NAT_DST "PRIO_NAT_DST" > > +%token PRIO_FILTER "PRIO_FILTER" > > +%token PRIO_SECURITY "PRIO_SECURITY" > > +%token PRIO_NAT_SRC "PRIO_NAT_SRC" > > +%token PRIO_SELINUX_LAST "PRIO_SELINUX_LAST" > > +%token PRIO_CONNTRACK_HELPER "PRIO_CONNTRACK_HELPER" > > We can probably handle this as strings, so we don't need to update > scanner.l Could you describe more what this means? I'm not clear about how scanner.l and parser_bison.y are connected and what their roles are accurately. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 04, 2018 at 01:30:45PM +0200, Pablo Neira Ayuso wrote: > On Mon, Jun 04, 2018 at 12:10:49PM +0200, Arturo Borrero Gonzalez wrote: > > On 4 June 2018 at 11:58, Máté Eckl <ecklm94@gmail.com> wrote: > > > What I'm not sure of is: > > > - Are these token values considered user-friendly or usable? > > > - Is printing of these values with their names desired? > > > > > > What do you think? > > > > > > -- 8< -- > > > This patch adds the possibility to use textual names to set the chain priority > > > to basic values so that numeric values do not need to be learnt any more for > > > basic usage. > > > > > > Example: > > > nft> add table inet x > > > nft> add chain inet x y {type filter hook prerouting priority PRIO_MANGLE ;} > > > nft> list ruleset > > > table inet x { > > > chain y { > > > type filter hook prerouting priority -150; policy accept; > > > } > > > } > > > > > > > I believe the idea is good. But also, you should print the friendly > > names instead of the magic numbers :-P > > Right, symmetry is desired thing in this. > > it would be cool if we could print based on approximate matching, eg. > > -150 would be printed as "mangle". > > but: > > -149 would be printed as "mangle + 1" > > so people could do arithmetics based on the tags. I didn't think this so deeply, but this is a nice idea. On the print side, however, there must be a border until which we print it like this. I mean mangle + 75 carries hardly more information than -75 (in this case, the user should have a clue about the actual values to know, what he/she is doing), so probably only values like mangle +- 10 should be printed like this. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/parser_bison.y b/src/parser_bison.y index 034dd01..236e9be 100644 --- a/src/parser_bison.y +++ b/src/parser_bison.y @@ -21,6 +21,7 @@ #include <linux/netfilter/nf_conntrack_tuple_common.h> #include <linux/netfilter/nf_nat.h> #include <linux/netfilter/nf_log.h> +#include <linux/netfilter_ipv4.h> #include <netinet/ip_icmp.h> #include <netinet/icmp6.h> #include <libnftnl/common.h> @@ -313,6 +314,19 @@ int nft_lex(void *, void *, void *); %token NEXTHDR "nexthdr" %token HOPLIMIT "hoplimit" +%token PRIO_RAW_BEFORE_DEFRAG "PRIO_RAW_BEFORE_DEFRAG" +%token PRIO_CONNTRACK_DEFRAG "PRIO_CONNTRACK_DEFRAG" +%token PRIO_RAW "PRIO_RAW" +%token PRIO_SELINUX_FIRST "PRIO_SELINUX_FIRST" +%token PRIO_CONNTRACK "PRIO_CONNTRACK" +%token PRIO_MANGLE "PRIO_MANGLE" +%token PRIO_NAT_DST "PRIO_NAT_DST" +%token PRIO_FILTER "PRIO_FILTER" +%token PRIO_SECURITY "PRIO_SECURITY" +%token PRIO_NAT_SRC "PRIO_NAT_SRC" +%token PRIO_SELINUX_LAST "PRIO_SELINUX_LAST" +%token PRIO_CONNTRACK_HELPER "PRIO_CONNTRACK_HELPER" + %token ICMP6 "icmpv6" %token PPTR "param-problem" %token MAXDELAY "max-delay" @@ -1790,8 +1804,20 @@ hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY prio_spec } ; -prio_spec : NUM { $$ = $1; } - | DASH NUM { $$ = -$2; } +prio_spec : NUM { $$ = $1; } + | DASH NUM { $$ = -$2; } + | PRIO_RAW_BEFORE_DEFRAG { $$ = NF_IP_PRI_RAW_BEFORE_DEFRAG; } + | PRIO_CONNTRACK_DEFRAG { $$ = NF_IP_PRI_CONNTRACK_DEFRAG; } + | PRIO_RAW { $$ = NF_IP_PRI_RAW; } + | PRIO_SELINUX_FIRST { $$ = NF_IP_PRI_SELINUX_FIRST; } + | PRIO_CONNTRACK { $$ = NF_IP_PRI_CONNTRACK; } + | PRIO_MANGLE { $$ = NF_IP_PRI_MANGLE; } + | PRIO_NAT_DST { $$ = NF_IP_PRI_NAT_DST; } + | PRIO_FILTER { $$ = NF_IP_PRI_FILTER; } + | PRIO_SECURITY { $$ = NF_IP_PRI_SECURITY; } + | PRIO_NAT_SRC { $$ = NF_IP_PRI_NAT_SRC; } + | PRIO_SELINUX_LAST { $$ = NF_IP_PRI_SELINUX_LAST;} + | PRIO_CONNTRACK_HELPER { $$ = NF_IP_PRI_CONNTRACK_HELPER; } ; dev_spec : DEVICE STRING { $$ = $2; } diff --git a/src/scanner.l b/src/scanner.l index 416bd27..846bd34 100644 --- a/src/scanner.l +++ b/src/scanner.l @@ -427,6 +427,19 @@ addrstring ({macaddr}|{ip4addr}|{ip6addr}) "nexthdr" { return NEXTHDR; } "hoplimit" { return HOPLIMIT; } +"PRIO_RAW_BEFORE_DEFRAG" { return PRIO_RAW_BEFORE_DEFRAG; } +"PRIO_CONNTRACK_DEFRAG" { return PRIO_CONNTRACK_DEFRAG; } +"PRIO_RAW" { return PRIO_RAW; } +"PRIO_SELINUX_FIRST" { return PRIO_SELINUX_FIRST; } +"PRIO_CONNTRACK" { return PRIO_CONNTRACK; } +"PRIO_MANGLE" { return PRIO_MANGLE; } +"PRIO_NAT_DST" { return PRIO_NAT_DST; } +"PRIO_FILTER" { return PRIO_FILTER; } +"PRIO_SECURITY" { return PRIO_SECURITY; } +"PRIO_NAT_SRC" { return PRIO_NAT_SRC; } +"PRIO_SELINUX_LAST" { return PRIO_SELINUX_LAST; } +"PRIO_CONNTRACK_HELPER" { return PRIO_CONNTRACK_HELPER; } + "icmpv6" { return ICMP6; } "param-problem" { return PPTR; } "max-delay" { return MAXDELAY; }
What I'm not sure of is: - Are these token values considered user-friendly or usable? - Is printing of these values with their names desired? What do you think? -- 8< -- This patch adds the possibility to use textual names to set the chain priority to basic values so that numeric values do not need to be learnt any more for basic usage. Example: nft> add table inet x nft> add chain inet x y {type filter hook prerouting priority PRIO_MANGLE ;} nft> list ruleset table inet x { chain y { type filter hook prerouting priority -150; policy accept; } } Signed-off-by: Máté Eckl <ecklm94@gmail.com> --- src/parser_bison.y | 30 ++++++++++++++++++++++++++++-- src/scanner.l | 13 +++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-)