| Message ID | 20190817111753.8756-2-a@juaristi.eus |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | None | expand |
The naming of the new meta keys seem a bit confusing. On Sat, Aug 17, 2019 at 1:19 PM Ander Juaristi <a@juaristi.eus> wrote: > > This patch introduces meta matches in the kernel for time (a UNIX timestamp), > day (a day of week, represented as an integer between 0-6), and > hour (an hour in the current day, or: number of seconds since midnight). > > All values are taken as unsigned 64-bit integers. > > The 'time' keyword is internally converted to nanoseconds by nft in > userspace, and hence the timestamp is taken in nanoseconds as well. > > Signed-off-by: Ander Juaristi <a@juaristi.eus> > --- > include/uapi/linux/netfilter/nf_tables.h | 6 ++++ > net/netfilter/nft_meta.c | 46 ++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > > diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h > index 82abaa183fc3..b83b62eb4b01 100644 > --- a/include/uapi/linux/netfilter/nf_tables.h > +++ b/include/uapi/linux/netfilter/nf_tables.h > @@ -799,6 +799,9 @@ enum nft_exthdr_attributes { > * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) > * @NFT_META_BRI_IIFPVID: packet input bridge port pvid > * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto > + * @NFT_META_TIME_NS: time since epoch (in nanoseconds) > + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday) This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a lot of things. Matches nicely with the added nft_meta_weekday function too. > + * @NFT_META_TIME_HOUR: hour of day (in seconds) This isn't really an hour, so why call it that (confuses unit at least)? Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY. Presumably the added nft_meta_hour function also derives its name from this, but otherwise has nothing to do with hours. > */ > enum nft_meta_keys { > NFT_META_LEN, ...
On 17/8/19 15:43, Jones Desougi wrote: >> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h >> index 82abaa183fc3..b83b62eb4b01 100644 >> --- a/include/uapi/linux/netfilter/nf_tables.h >> +++ b/include/uapi/linux/netfilter/nf_tables.h >> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes { >> * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) >> * @NFT_META_BRI_IIFPVID: packet input bridge port pvid >> * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto >> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds) >> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday) > > This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a > lot of things. > Matches nicely with the added nft_meta_weekday function too. I agree with you here. Seems to me WEEKDAY is clearer. > >> + * @NFT_META_TIME_HOUR: hour of day (in seconds) > > This isn't really an hour, so why call it that (confuses unit at least)? > Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY. > Presumably the added nft_meta_hour function also derives its name from > this, but otherwise has nothing to do with hours. > But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR, though less explicit (more ambiguous). >> */ >> enum nft_meta_keys { >> NFT_META_LEN, > ... >
On Sun, Aug 18, 2019 at 8:22 PM Ander Juaristi <a@juaristi.eus> wrote: > > On 17/8/19 15:43, Jones Desougi wrote: > >> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h > >> index 82abaa183fc3..b83b62eb4b01 100644 > >> --- a/include/uapi/linux/netfilter/nf_tables.h > >> +++ b/include/uapi/linux/netfilter/nf_tables.h > >> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes { > >> * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) > >> * @NFT_META_BRI_IIFPVID: packet input bridge port pvid > >> * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto > >> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds) > >> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday) > > > > This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a > > lot of things. > > Matches nicely with the added nft_meta_weekday function too. > > I agree with you here. Seems to me WEEKDAY is clearer. > > > > >> + * @NFT_META_TIME_HOUR: hour of day (in seconds) > > > > This isn't really an hour, so why call it that (confuses unit at least)? > > Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY. > > Presumably the added nft_meta_hour function also derives its name from > > this, but otherwise has nothing to do with hours. > > > > But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR, > though less explicit (more ambiguous). HOUR is a unit, much like NS, but its use is quite different with no clear hint as to how. Unlike the latter it's also not the unit of the value. From that perspective the name comes up empty of meaning. If you already know what it means, the name can be put in context, but that's not explicit at all. That said, weekday is more important. > > >> */ > >> enum nft_meta_keys { > >> NFT_META_LEN, > > ... > > > >
Jones Desougi <jones.desougi+netfilter@gmail.com> wrote: > On Sun, Aug 18, 2019 at 8:22 PM Ander Juaristi <a@juaristi.eus> wrote: > > > > On 17/8/19 15:43, Jones Desougi wrote: > > >> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h > > >> index 82abaa183fc3..b83b62eb4b01 100644 > > >> --- a/include/uapi/linux/netfilter/nf_tables.h > > >> +++ b/include/uapi/linux/netfilter/nf_tables.h > > >> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes { > > >> * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) > > >> * @NFT_META_BRI_IIFPVID: packet input bridge port pvid > > >> * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto > > >> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds) > > >> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday) > > > > > > This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a > > > lot of things. > > > Matches nicely with the added nft_meta_weekday function too. > > > > I agree with you here. Seems to me WEEKDAY is clearer. > > > > > > > >> + * @NFT_META_TIME_HOUR: hour of day (in seconds) > > > > > > This isn't really an hour, so why call it that (confuses unit at least)? > > > Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY. > > > Presumably the added nft_meta_hour function also derives its name from > > > this, but otherwise has nothing to do with hours. > > > > > > > But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR, > > though less explicit (more ambiguous). > > HOUR is a unit, much like NS, but its use is quite different with no > clear hint as to how. Unlike the latter it's also not the unit of the > value. From that perspective the name comes up empty of meaning. If > you already know what it means, the name can be put in context, but > that's not explicit at all. If the NFT_META_TIME_* names are off, then those for the frontend are too. I think meta time <iso-date> meta hour <relative to this day> meta day <weekday> are fine, and thus so are the uapi enums. Examples: meta time < "2019-06-06 17:20:20" drop meta hour 11:00-17:00 accept meta day "Sat" drop What would you suggest as alternatives?
On Sat, Aug 17, 2019 at 01:17:53PM +0200, Ander Juaristi wrote: > This patch introduces meta matches in the kernel for time (a UNIX timestamp), > day (a day of week, represented as an integer between 0-6), and > hour (an hour in the current day, or: number of seconds since midnight). > > All values are taken as unsigned 64-bit integers. > > The 'time' keyword is internally converted to nanoseconds by nft in > userspace, and hence the timestamp is taken in nanoseconds as well. Also applied, thanks.
Sorry, been away. On Tue, Aug 20, 2019 at 9:27 PM Florian Westphal <fw@strlen.de> wrote: > > Jones Desougi <jones.desougi+netfilter@gmail.com> wrote: > > On Sun, Aug 18, 2019 at 8:22 PM Ander Juaristi <a@juaristi.eus> wrote: > > > > > > On 17/8/19 15:43, Jones Desougi wrote: > > > >> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h > > > >> index 82abaa183fc3..b83b62eb4b01 100644 > > > >> --- a/include/uapi/linux/netfilter/nf_tables.h > > > >> +++ b/include/uapi/linux/netfilter/nf_tables.h > > > >> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes { > > > >> * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) > > > >> * @NFT_META_BRI_IIFPVID: packet input bridge port pvid > > > >> * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto > > > >> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds) > > > >> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday) > > > > > > > > This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a > > > > lot of things. > > > > Matches nicely with the added nft_meta_weekday function too. > > > > > > I agree with you here. Seems to me WEEKDAY is clearer. > > > > > > > > > > >> + * @NFT_META_TIME_HOUR: hour of day (in seconds) > > > > > > > > This isn't really an hour, so why call it that (confuses unit at least)? > > > > Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY. > > > > Presumably the added nft_meta_hour function also derives its name from > > > > this, but otherwise has nothing to do with hours. > > > > > > > > > > But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR, > > > though less explicit (more ambiguous). > > > > HOUR is a unit, much like NS, but its use is quite different with no > > clear hint as to how. Unlike the latter it's also not the unit of the > > value. From that perspective the name comes up empty of meaning. If > > you already know what it means, the name can be put in context, but > > that's not explicit at all. > > If the NFT_META_TIME_* names are off, then those for the > frontend are too. In part, but they are not that tightly bound. Or shouldn't be. See regarding hour below. > > I think > meta time <iso-date> > meta hour <relative to this day> > meta day <weekday> > > are fine, and thus so are the uapi enums. > > Examples: > > meta time < "2019-06-06 17:20:20" drop > meta hour 11:00-17:00 accept > meta day "Sat" drop > > What would you suggest as alternatives? Using weekday here as well would be the obvious choice, e.g. meta weekday "Sat" drop Not being explicit seems bad to me, especially in the kernel interface. However, for hour the context is different. Hour is relevant in this frontend interface, since the time specification is hour anchored. An alternative implementation might use nautic 'five bells in morning watch' or whatever for the user, but it would still translate to time of day in seconds in the kernel interface.
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h index 82abaa183fc3..b83b62eb4b01 100644 --- a/include/uapi/linux/netfilter/nf_tables.h +++ b/include/uapi/linux/netfilter/nf_tables.h @@ -799,6 +799,9 @@ enum nft_exthdr_attributes { * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) * @NFT_META_BRI_IIFPVID: packet input bridge port pvid * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto + * @NFT_META_TIME_NS: time since epoch (in nanoseconds) + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday) + * @NFT_META_TIME_HOUR: hour of day (in seconds) */ enum nft_meta_keys { NFT_META_LEN, @@ -831,6 +834,9 @@ enum nft_meta_keys { NFT_META_OIFKIND, NFT_META_BRI_IIFPVID, NFT_META_BRI_IIFVPROTO, + NFT_META_TIME_NS, + NFT_META_TIME_DAY, + NFT_META_TIME_HOUR, }; /** diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c index f1b1d948c07b..096b831e3758 100644 --- a/net/netfilter/nft_meta.c +++ b/net/netfilter/nft_meta.c @@ -26,8 +26,36 @@ #include <uapi/linux/netfilter_bridge.h> /* NF_BR_PRE_ROUTING */ +#define NFT_META_SECS_PER_MINUTE 60 +#define NFT_META_SECS_PER_HOUR 3600 +#define NFT_META_SECS_PER_DAY 86400 +#define NFT_META_DAYS_PER_WEEK 7 + static DEFINE_PER_CPU(struct rnd_state, nft_prandom_state); +static u8 nft_meta_weekday(unsigned long secs) +{ + unsigned int dse; + u8 wday; + + secs -= NFT_META_SECS_PER_MINUTE * sys_tz.tz_minuteswest; + dse = secs / NFT_META_SECS_PER_DAY; + wday = (4 + dse) % NFT_META_DAYS_PER_WEEK; + + return wday; +} + +static u32 nft_meta_hour(unsigned long secs) +{ + struct tm tm; + + time64_to_tm(secs, 0, &tm); + + return tm.tm_hour * NFT_META_SECS_PER_HOUR + + tm.tm_min * NFT_META_SECS_PER_MINUTE + + tm.tm_sec; +} + void nft_meta_get_eval(const struct nft_expr *expr, struct nft_regs *regs, const struct nft_pktinfo *pkt) @@ -226,6 +254,15 @@ void nft_meta_get_eval(const struct nft_expr *expr, goto err; strncpy((char *)dest, out->rtnl_link_ops->kind, IFNAMSIZ); break; + case NFT_META_TIME_NS: + nft_reg_store64(dest, ktime_get_real_ns()); + break; + case NFT_META_TIME_DAY: + nft_reg_store8(dest, nft_meta_weekday(get_seconds())); + break; + case NFT_META_TIME_HOUR: + *dest = nft_meta_hour(get_seconds()); + break; default: WARN_ON(1); goto err; @@ -338,6 +375,15 @@ int nft_meta_get_init(const struct nft_ctx *ctx, len = sizeof(u8); break; #endif + case NFT_META_TIME_NS: + len = sizeof(u64); + break; + case NFT_META_TIME_DAY: + len = sizeof(u8); + break; + case NFT_META_TIME_HOUR: + len = sizeof(u32); + break; default: return -EOPNOTSUPP; }
This patch introduces meta matches in the kernel for time (a UNIX timestamp), day (a day of week, represented as an integer between 0-6), and hour (an hour in the current day, or: number of seconds since midnight). All values are taken as unsigned 64-bit integers. The 'time' keyword is internally converted to nanoseconds by nft in userspace, and hence the timestamp is taken in nanoseconds as well. Signed-off-by: Ander Juaristi <a@juaristi.eus> --- include/uapi/linux/netfilter/nf_tables.h | 6 ++++ net/netfilter/nft_meta.c | 46 ++++++++++++++++++++++++ 2 files changed, 52 insertions(+)