On Fri, Jul 20, 2018 at 3:21 PM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Thu, Jul 19, 2018 at 03:10:14PM +0200, Harsha Sharma wrote:
>> On Thu, Jul 19, 2018 at 2:33 AM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> [...]
>> >> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> >> index 3f211e1025c1..c1cf24b6db96 100644
>> >> --- a/net/netfilter/nf_tables_api.c
>> >> +++ b/net/netfilter/nf_tables_api.c
>> >> @@ -7152,6 +7152,10 @@ static int __net_init nf_tables_init_net(struct net *net)
>> >> {
>> >> INIT_LIST_HEAD(&net->nft.tables);
>> >> INIT_LIST_HEAD(&net->nft.commit_list);
>> >> +
>> >> +#if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
>> >
>> > Probably better:
>> >
>> > #if IS_ENABLED(CONFIG_NF_CONNTRACK_TIMEOUT)
>>
>> CONFIG_NF_CT_NETLINK_TIMEOUT is required for struct
>> nf_conntrack_l4proto to have a member struct ctnl_timeout.
>
> No. A structure definition doesn't create a dependency that would
> break things.
>
> You just have to include the header file and use it, that's all.
With CONFIG_NF_CT_NETLINK_TIMEOUT = n and CONFIG_NF_CONNTRACK_TIMEOUT
= y, it doesn't compile properly if I change it to
IS_ENABLED(CONFIG_NF_CONNTRACK_TIMEOUT) in nft_ct.c with error "‘const
struct nf_conntrack_l4proto’ has no member named ‘ctnl_timeout’".
Do you still want me to change it here ?
Thank you very much.
> Problems are function calls, those are real dependencies between
> modules.
>
>> Since, NF_CT_NETLINK_TIMEOUT already depends on NF_CONNTRACK_CORE, it
>> will make sense to change it in nf_conntrack_l4proto.h#L20.
>> Do you also want to change this in files like nf_conntrack_proto_tcp.c ?
>
> Not really, those are really only useful for NF_CT_NETLINK_TIMEOUT.
>
> Why do you want to update those?
>
> [...]
>> >> +static void nft_ct_timeout_obj_eval(struct nft_object *obj,
>> >> + struct nft_regs *regs,
>> >> + const struct nft_pktinfo *pkt)
>> >> +{
>> >> + const struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
>> >> + struct ctnl_timeout *to_assign = NULL;
>> >> + struct nf_conn_timeout *timeout_ext;
>> >> + struct sk_buff *skb = pkt->skb;
>> >> + enum ip_conntrack_info ctinfo;
>> >> +
>> >> + if (nf_ct_get(skb, &ctinfo))
>> >> + return;
>> >> +
>> >> + to_assign = priv->timeout;
>> >> + timeout_ext = nf_ct_timeout_find(priv->tmpl);
>> >
>> > This two lines above.
>> >
>> >> + nf_ct_set(skb, priv->tmpl, IP_CT_NEW);
>> >> + rcu_assign_pointer(timeout_ext->timeout, to_assign);
>> >
>> > And this one above... belong to the nft_ct_timeout_obj_init() path.
>> >
>> > So, only nf_ct_set(skb, ...) is sufficient to set the custom timeout,
>> > if the tmpl object is correct initialization from the init path.
>>
>> I'll do the other changes. thanks.
>
> Thanks.
--
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
