netdev - Re: [PATCH net-next v2 1/3] net/sched: act

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210530114716.GA16534@builder>
Date:   Sun, 30 May 2021 14:47:16 +0300
From:   Boris Sukholitko <boris.sukholitko@...adcom.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     netdev@...r.kernel.org, Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Ilya Lifshits <ilya.lifshits@...adcom.com>,
        Shmulik Ladkani <shmulik.ladkani@...il.com>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next v2 1/3] net/sched: act_vlan: Fix modify to allow
 0

On Thu, May 27, 2021 at 07:00:52PM +0200, Davide Caratti wrote:
[...]
> 
> My suggestion was just to simplify the end-user dump experience, so
> that the value of 'p->tcfv_push_prio' is dumped always in case of
> TCA_VLAN_ACT_PUSH. In this way, rules with equal "behavior" in the
> traffic path are always dumped in the same way. IOW,
> 
> # tc action add action vlan push id 42 prio 0 index 1
> 
> and
> 
> # tc action add action vlan push id 42 index 1
> 
> do exactly the same thing in the traffic path, so there is no need to
> dump them differently. On the contrary, these 2 rules:
> 
> # tc action add action vlan modify id 42 prio 0 index 1
> 
> and
> 
> # tc action add action vlan modify id 42 index 1
> 
> don't do the same thing, because packet hitting the first rule will have
> their priority identically set to 0, while the second one will leave the
> VLAN priority unmodified. So, I think it makes sense to have different
> dumps here (that was my comment to your v1).

I am convinced. I've done this in v3.

> 
> Another small nit - forgive me, I didn't spot it in the first review:
> 
> not 100% sure, but I think that in tcf_vlan_get_fill_size() we need
> to avoid accounting for TCA_VLAN_PUSH_VLAN_PRIORITY in case the rule
> has 'push_prio_exists' equal to false. Otherwise we allocate an
> extra u8 netlink attribute in case of batch dump. Correct?

Also done in v3.

Thanks,
Boris.

> 
> thanks!
> -- 
> davide
> 
> 
> 

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)