netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: wenxu <wenxu@ucloud.cn>
To: Florian Westphal <fw@strlen.de>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>,
	netfilter-devel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH] netfilter: nft_paylaod: add base type NFT_PAYLOAD_LL_HEADER_NO_TAG
Date: Tue, 18 Jun 2019 22:27:12 +0800	[thread overview]
Message-ID: <591caf69-ba08-33b5-5330-8230779cc903@ucloud.cn> (raw)
In-Reply-To: <20190618093748.dydodhngydfcfmeh@breakpoint.cc>


在 2019/6/18 17:37, Florian Westphal 写道:
> wenxu <wenxu@ucloud.cn> wrote:
>> On 6/18/2019 6:42 AM, Florian Westphal wrote:
>>> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>>>> Subject: Change bridge l3 dependency to meta protocol
>>>>>
>>>>> This examines skb->protocol instead of ethernet header type, which
>>>>> might be different when vlan is involved.
>>>>>  
>>>>> +	if (ctx->pctx.family == NFPROTO_BRIDGE && desc == &proto_eth) {
>>>>> +		if (expr->payload.desc == &proto_ip ||
>>>>> +		    expr->payload.desc == &proto_ip6)
>>>>> +			desc = &proto_metaeth;
>>>>> +	}i
>>>> Is this sufficient to restrict the matching? Is this still buggy from
>>>> ingress?
>>> This is what netdev family uses as well (skb->protocol i mean).
>>> I'm not sure it will work for output however (haven't checked).
>>>
>>>> I wonder if an explicit NFT_PAYLOAD_CHECK_VLAN flag would be useful in
>>>> the kernel, if so we could rename NFTA_PAYLOAD_CSUM_FLAGS to
>>>> NFTA_PAYLOAD_FLAGS and place it there. Just an idea.
>>> Another unresolved issue is presence of multiple vlan tags, so we might
>>> have to add yet another meta key to retrieve the l3 protocol in use
>> Maybe add a l3proto meta key can handle the multiple vlan tags case with the l3proto dependency.  It
>> should travese all the vlan tags and find the real l3 proto.
> Yes, something like this.
>
> We also need to audit netdev and bridge expressions (reject is known broken)
> to handle vlans properly.
>
> Still, switching nft to prefer skb->protocol instead of eth_hdr->type
> for dependencies would be good as this doesn't need kernel changes and solves
> the immediate problem of 'ip ...' not matching in case of vlan.
>
> If you have time, could you check if using skb->protocol works for nft
> bridge in output, i.e. does 'nft ip protocol icmp' match when its used
> from bridge output path with meta protocol dependency with and without
> vlan in use?

I just check the kernel codes and test with the output chain, the meta protocol dependency can

also work in the outpu chain.

But this patch can't resolve the multiple vlan tags, It need another meta l3proto which do care about

how many vlan tags in the frame.


  reply	other threads:[~2019-06-18 14:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-10  7:21 [PATCH] netfilter: nft_paylaod: add base type NFT_PAYLOAD_LL_HEADER_NO_TAG wenxu
2019-06-10  9:44 ` Florian Westphal
2019-06-11  3:01   ` wenxu
2019-06-17 22:30   ` Pablo Neira Ayuso
2019-06-17 22:42     ` Florian Westphal
2019-06-18  8:26       ` wenxu
2019-06-18  9:37         ` Florian Westphal
2019-06-18 14:27           ` wenxu [this message]
2019-06-18 15:33             ` Pablo Neira Ayuso
2019-06-18  9:35       ` Pablo Neira Ayuso
2019-06-18  9:46         ` Florian Westphal
2019-06-18 10:04           ` Pablo Neira Ayuso
2019-06-18 10:45             ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=591caf69-ba08-33b5-5330-8230779cc903@ucloud.cn \
    --to=wenxu@ucloud.cn \
    --cc=fw@strlen.de \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).