Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > So I'm not sure its right to extend nftrace with additional selectors (its > > also possible that I failed to understand what you were suggesting 8-} ) > > I can see room to extend this infrastructure later on if we need to > make it more flexible (eg. allow the user to specify what part of the > packets need to be dumped to userspace). We can probably even add a > specific 'trace' expression from the kernel to allow more specific > selection on what packet fields you want to dump to userspace. > > BTW, any reason to remove the existing infrastructure? It's been there > since almost the beginning (this would break users that are expecting > the existing behaviour). No specific reason, but I question the need to keep it around. Once we have native tooling there should be no reason whatsover to wade through dmesg output + manual matching of rules... its just a PITA and thats why I removed it -- 'nft monitor trace' (or whatever, I've not decided on what nft side should look like) would be much simpler/easier to use. > Moreover, we'll have people using the > iptables-compat infrastructure also expecting a similar output to > native iptables. Hmm, is iptables-compat a worthy focus? AFAIU even iptables-compat users would be able to use nft trace infrastructure, right? Wouldn't it make more sense to convince people to go with real nft rather than the compat layer? > You can probably generalize the trace infrastructure through static > key plus indirections to keep both tracing approaches around (allowing > only one at the same time should be enough). I'd like to see a stronger argument than this. Bottom line is that i want to keep kernel side as simple as possible, so I'd much rather just get rid of the printk-based tracing (I'm *NOT* talking about the ip(6)tables stuff, that remains as-is) rather than add more conditionals or indirect calls to some trace-backends... > From userspace, we would need to have a way to indicate that we want > to use the new infrastructure, not the classic tracing. That syntax > would need some thinking. Probably we can introduce a 'trace' > statement, so the syntax looks compact like this: > > 'tcp port 22 limit rate 1/second trace' > > Meaning in this case that the userspace is specifically requesting for > the new kind of tracing. Hmm. I'm not convinced there is any gain in doing this. Who in their right mind would want to manually decode rules...? That being said, I am not overly zealous about this, and I can keep the printk stuff around. I just think its not needed anymore. If you have new nft with old kernel: - you need to look and dmesg output (or syslog output if ypu use nfnetlink_log logging backend). if you have new nft with new kernel: - nft monitor trace (subject to change, I'm open to suggestions) If you have old nft with new kernel: - possibly a problem, but then again, if you can update kernel, why not use a newer nft as well...? If you don't mind, i would prefer to work on this patch + nft + libnftables integration and then submit that formally. If you're then still convinced that its too problematic to remove the printk based trace i'd consider to rework the kernel patch. [ I'm not in a hurry, we can discuss this at netdev/netconf if you like ] Let me know. Thanks && regards, Florian -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
