On Tuesday 2012-02-14 11:47, Pablo Neira Ayuso wrote:
>> include/linux/netfilter/nfnetlink_xtables.h | 20 ++++
>> net/netfilter/Kconfig | 8 ++
>> net/netfilter/Makefile | 1 +
>> net/netfilter/xt2_nfnetlink.c | 153 +++++++++++++++++++++++++++
>
>I prefer if you call this nfnetlink_xtables.c following the same
>naming convention.
(I dropped the '2' meanwhile) Hm I chose so as I planned to have
at least another file, xt_compat.c. In that regard, it's more like
what ipset/ has (also not nfnetlink_ipset). Should I pick a
different header name then?
>> +enum nfxt_msg_type {
>> + NFXTM_IDENTIFY = 1,
>
>I'd suggest: NFXT_MSG_NEW, NFXT_MSG_GET,...and so on.
The identifiers are already long enough IMO. Hence the choice
to use NFXTM_ (msg), NFXTA_ (attr) and NFXTE_ (error codes).
>> +static struct nlmsghdr *
>> +xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
>> + unsigned int flags)
>> +{ [...]
>> + if (nlmsg == NULL) {
>> + nlmsg_cancel(skb, nlmsg);
>> + return ERR_PTR(-ENOBUFS);
>
>my experience is that it's better to use goto to handle errors.
There is just one error case here in this function,
so goto was nowhere to be desired :)
>> +static int
>> +xtnetlink_identify(struct sock *xtnl, struct sk_buff *iskb,
>> + const struct nlmsghdr *imsg, const struct nlattr *const *ad)
>> +{
>> + return netlink_dump_start(xtnl, iskb, imsg, xtnetlink_identify2,
>> + NULL, 0);
>
>you have to check for NLM_F_DUMP. Otherwise, you are requesting one
>single table. Please, see ctnetlink or nfnetlink_acct for reference.
identify returns auxiliary information about the implementation.
For tables there is(will be) NFXTM_TABLE_DUMP.
--
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
