Hi Pablo, On Thu, Jan 21, 2021 at 03:59:34PM +0100, Pablo Neira Ayuso wrote: > On Thu, Jan 21, 2021 at 03:57:59PM +0100, Pablo Neira Ayuso wrote: > > Hi Phil, > > > > On Thu, Jan 21, 2021 at 03:44:14PM +0100, Phil Sutter wrote: > > > Hi! > > > > > > On Thu, Jan 21, 2021 at 02:55:08PM +0100, Florian Westphal wrote: > > > > The tests fail because json printing omits a burst of 5 and > > > > the parser treats that as 'burst 0'. > > > > > > While this patch is correct in that it aligns json and bison parser > > > behaviours, I think omitting burst value in JSON output is a bug by > > > itself: We don't care about output length and users are supposed to > > > parse (and thus filter) the information anyway, so there's no gain from > > > omitting such info. I'll address this in a separate patch, though. > > > > The listing of: > > > > nft list ruleset > > > > is already omitting this. Would you prefer this is also exposed there? > > I mean, IIRC for json it makes sense to display every field (not omit > anything), so my question is whether you think the native syntax > should omit this or it's fine as it is. You hit the bull's eye: I have a ticket about this behaviour already, claiming that having a non-trivial default value and omitting it from output is not a good idea. In practice, reporter created a limit statement which doesn't work with default burst value (limit rate 1). I'm not against omitting the burst, but it must not become a problem then. So my idea to keep the benefits of both was to implement an "auto burst value" which adjusts to the rate value. Do you think that's feasible? Maybe something simple like 'burst = max(1, rate / 10)' (for packets). Cheers, Phil
