Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Boris Sukholitko <boris.sukholitko@broadcom.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: netdev@vger.kernel.org, Jamal Hadi Salim <jhs@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Vadym Kochan <vadym.kochan@plvision.eu>,
	Ilya Lifshits <ilya.lifshits@broadcom.com>
Subject: Re: [PATCH net-next] net/sched: cls_flower: Add orig_ethtype
Date: Mon, 30 Aug 2021 12:42:40 +0300	[thread overview]
Message-ID: <20210830094240.GB24951@noodle> (raw)
In-Reply-To: <20210830092128.he5itvsbysvbaa5u@skbuf>

[-- Attachment #1: Type: text/plain, Size: 2727 bytes --]

On Mon, Aug 30, 2021 at 12:21:28PM +0300, Vladimir Oltean wrote:
> On Mon, Aug 30, 2021 at 12:18:13PM +0300, Boris Sukholitko wrote:
> > Hi Vladimir,
> >
> > On Mon, Aug 30, 2021 at 12:00:03PM +0300, Vladimir Oltean wrote:
> > [snip]
> > >
> > > It is very good that you've followed up this discussion with a patch:
> > > https://patchwork.kernel.org/project/netdevbpf/patch/20210617161435.8853-1-vadym.kochan@plvision.eu/
> > >
> > > I don't seem to see, however, in that discussion, what was the reasoning
> > > that led to the introduction of a new TCA_FLOWER_KEY_ORIG_ETH_TYPE as
> > > opposed to using TCA_FLOWER_KEY_ETH_TYPE?
> >
> > While trying to implement the plan from:
> >
> > https://patchwork.kernel.org/project/netdevbpf/patch/20210617161435.8853-1-vadym.kochan@plvision.eu/#24263965
> >
> > I've came upon the conclusion that it is better to make new orig_ethtype key
> > rather than reusing TCA_FLOWER_KEY_ETH_TYPE name. The changes I've
> > proposed there seem of a dubious value now. IMHO, of course :)
> >
> > >
> > > Can you explain in English what is the objective meaning of
> > > TCA_FLOWER_KEY_ORIG_ETH_TYPE, other than "what I need to solve my problem"?
> >
> > The orig part in the name means that the match is done on the original
> > protocol field of the packet, before dissector manipulation.
> >
> > > Maybe an entry in the man page section in your iproute2 patch?
> >
> > Yes, sure, good catch! I'll send V2 of the iproute2 patch shortly.
> >
> > >
> > > How will the VLAN case be dealt with? What is the current status quo on
> > > vlan_ethtype, will a tc-flower key of "vlan_ethtype $((ETH_P_PPP_SES))"
> > > match a VLAN-tagged PPP session packet or not, will the flow dissector
> > > still drill deep inside the packet? I guess this is the reason why you
> > > introduced another variant of the ETH_TYPE netlink attribute, to be
> > > symmetric with what could be done for VLAN? But I don't see VLAN changes?
> >
> > For VLAN, I intend to add [c]vlan_orig_ethtype keys. I intend to send those
> > (to-be-written :)) patches separately.
> 
> Wait a minute, don't hurry! We haven't even discussed offloading.
> So if I am writing a driver which offloads tc-flower, do I match on
> ETH_TYPE or on ORIG_ETH_TYPE? To me, the EtherType is, well, the EtherType...

AFAIK, the offloads are using basic.n_proto key now. This means matching
on the innermost protocol (i.e. after stripping various tunnels, vlan
etc.). Notice, how the offload driver has no access to the original
'protocol' setting.

ORIG_ETH_TYPE if given, asks to match on the original protocol as it
appears in the unmodified packet. This gives the offload driver writers
ability to match on it if the need arises.

Thanks,
Boris.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

  reply	other threads:[~2021-08-30  9:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-30  8:08 Boris Sukholitko
2021-08-30  9:00 ` Vladimir Oltean
2021-08-30  9:04   ` Vladimir Oltean
2021-08-30  9:18   ` Boris Sukholitko
2021-08-30  9:21     ` Vladimir Oltean
2021-08-30  9:42       ` Boris Sukholitko [this message]
2021-08-30 10:13         ` Vladimir Oltean
2021-08-31  1:48 ` Jamal Hadi Salim
2021-08-31 12:04   ` Boris Sukholitko
2021-08-31 13:18     ` Jamal Hadi Salim
2021-08-31 14:03       ` Boris Sukholitko
2021-09-02  6:48       ` Ido Schimmel
2021-09-03 22:52         ` Jamal Hadi Salim
2021-09-04 14:08       ` Tom Herbert

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=20210830094240.GB24951@noodle \
    --to=boris.sukholitko@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=ilya.lifshits@broadcom.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vadym.kochan@plvision.eu \
    --cc=xiyou.wangcong@gmail.com \
    --subject='Re: [PATCH net-next] net/sched: cls_flower: Add orig_ethtype' \
    /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

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).