[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241019083040.GI1697@kernel.org>
Date: Sat, 19 Oct 2024 09:30:40 +0100
From: Simon Horman <horms@...nel.org>
To: Yajun Deng <yajun.deng@...ux.dev>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, Ido Schimmel <idosch@...sch.org>,
Guillaume Nault <gnault@...hat.com>
Subject: Re: [PATCH v3 net-next] net: vlan: Use vlan_prio instead of vlan_qos
in mapping
+ Ido and Guilliame
On Fri, Oct 18, 2024 at 10:12:33PM +0800, Yajun Deng wrote:
> The vlan_qos member is used to save the vlan qos, but we only save the
> priority. Also, we will get the priority in vlan netlink and proc.
> We can just save the vlan priority using vlan_prio, so we can use vlan_prio
> to get the priority directly.
>
> For flexibility, we introduced vlan_dev_get_egress_priority() helper
> function. After this patch, we will call vlan_dev_get_egress_priority()
> instead of vlan_dev_get_egress_qos_mask() in irdma.ko and rdma_cm.ko.
> Because we don't need the shift and mask operations anymore.
>
> There is no functional changes.
>
> Signed-off-by: Yajun Deng <yajun.deng@...ux.dev>
Hi Ido and Guilliame,
I'm wondering if you could take a look over this and provide
any feedback you might have.
Thanks!
> ---
> v3: Remove the restriction that the maximum vlan priority is 7.
> v2: Add more detailed comments and tests.
v2 is here: https://lore.kernel.org/netdev/20241012132826.2224-1-yajun.deng@linux.dev/
> v1: https://lore.kernel.org/all/20241009132302.2902-1-yajun.deng@linux.dev/
> ---
> include/linux/if_vlan.h | 26 +++++++++++++++++++-------
> net/8021q/vlan.h | 4 ++--
> net/8021q/vlan_dev.c | 23 ++++++++++++-----------
> net/8021q/vlan_netlink.c | 4 ++--
> net/8021q/vlanproc.c | 4 ++--
> 5 files changed, 37 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index c1645c86eed9..7cc36853c017 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -150,12 +150,12 @@ extern __be16 vlan_dev_vlan_proto(const struct net_device *dev);
> /**
> * struct vlan_priority_tci_mapping - vlan egress priority mappings
> * @priority: skb priority
> - * @vlan_qos: vlan priority: (skb->priority << 13) & 0xE000
> + * @vlan_prio: vlan priority
> * @next: pointer to next struct
> */
> struct vlan_priority_tci_mapping {
> u32 priority;
> - u16 vlan_qos;
> + u8 vlan_prio;
> struct vlan_priority_tci_mapping *next;
> };
>
> @@ -204,8 +204,8 @@ static inline struct vlan_dev_priv *vlan_dev_priv(const struct net_device *dev)
> return netdev_priv(dev);
> }
>
> -static inline u16
> -vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
> +static inline u8
> +vlan_dev_get_egress_priority(struct net_device *dev, u32 skprio)
> {
> struct vlan_priority_tci_mapping *mp;
>
> @@ -214,15 +214,21 @@ vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
> mp = vlan_dev_priv(dev)->egress_priority_map[(skprio & 0xF)];
> while (mp) {
> if (mp->priority == skprio) {
> - return mp->vlan_qos; /* This should already be shifted
> - * to mask correctly with the
> - * VLAN's TCI */
> + return mp->vlan_prio;
> }
> mp = mp->next;
> }
> return 0;
> }
>
> +static inline u16
> +vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
> +{
> + u8 vlan_prio = vlan_dev_get_egress_priority(dev, skprio);
> +
> + return (vlan_prio << VLAN_PRIO_SHIFT) & VLAN_PRIO_MASK;
> +}
> +
> extern bool vlan_do_receive(struct sk_buff **skb);
>
> extern int vlan_vid_add(struct net_device *dev, __be16 proto, u16 vid);
> @@ -269,6 +275,12 @@ static inline __be16 vlan_dev_vlan_proto(const struct net_device *dev)
> return 0;
> }
>
> +static inline u8 vlan_dev_get_egress_priority(struct net_device *dev,
> + u32 skprio)
> +{
> + return 0;
> +}
> +
> static inline u16 vlan_dev_get_egress_qos_mask(struct net_device *dev,
> u32 skprio)
> {
> diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
> index 5eaf38875554..b28875c4ac86 100644
> --- a/net/8021q/vlan.h
> +++ b/net/8021q/vlan.h
> @@ -126,9 +126,9 @@ void vlan_filter_drop_vids(struct vlan_info *vlan_info, __be16 proto);
>
> /* found in vlan_dev.c */
> void vlan_dev_set_ingress_priority(const struct net_device *dev,
> - u32 skb_prio, u16 vlan_prio);
> + u32 skb_prio, u8 vlan_prio);
> int vlan_dev_set_egress_priority(const struct net_device *dev,
> - u32 skb_prio, u16 vlan_prio);
> + u32 skb_prio, u8 vlan_prio);
> void vlan_dev_free_egress_priority(const struct net_device *dev);
> int vlan_dev_change_flags(const struct net_device *dev, u32 flag, u32 mask);
> void vlan_dev_get_realdev_name(const struct net_device *dev, char *result,
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 458040e8a0e0..5d153f1a7963 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -155,35 +155,36 @@ static int vlan_dev_change_mtu(struct net_device *dev, int new_mtu)
> }
>
> void vlan_dev_set_ingress_priority(const struct net_device *dev,
> - u32 skb_prio, u16 vlan_prio)
> + u32 skb_prio, u8 vlan_prio)
> {
> struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>
> - if (vlan->ingress_priority_map[vlan_prio & 0x7] && !skb_prio)
> + vlan_prio = vlan_prio & 0x7;
> + if (vlan->ingress_priority_map[vlan_prio] && !skb_prio)
> vlan->nr_ingress_mappings--;
> - else if (!vlan->ingress_priority_map[vlan_prio & 0x7] && skb_prio)
> + else if (!vlan->ingress_priority_map[vlan_prio] && skb_prio)
> vlan->nr_ingress_mappings++;
>
> - vlan->ingress_priority_map[vlan_prio & 0x7] = skb_prio;
> + vlan->ingress_priority_map[vlan_prio] = skb_prio;
> }
>
> int vlan_dev_set_egress_priority(const struct net_device *dev,
> - u32 skb_prio, u16 vlan_prio)
> + u32 skb_prio, u8 vlan_prio)
> {
> struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> struct vlan_priority_tci_mapping *mp = NULL;
> struct vlan_priority_tci_mapping *np;
> - u32 vlan_qos = (vlan_prio << VLAN_PRIO_SHIFT) & VLAN_PRIO_MASK;
>
> + vlan_prio = vlan_prio & 0x7;
> /* See if a priority mapping exists.. */
> mp = vlan->egress_priority_map[skb_prio & 0xF];
> while (mp) {
> if (mp->priority == skb_prio) {
> - if (mp->vlan_qos && !vlan_qos)
> + if (mp->vlan_prio && !vlan_prio)
> vlan->nr_egress_mappings--;
> - else if (!mp->vlan_qos && vlan_qos)
> + else if (!mp->vlan_prio && vlan_prio)
> vlan->nr_egress_mappings++;
> - mp->vlan_qos = vlan_qos;
> + mp->vlan_prio = vlan_prio;
> return 0;
> }
> mp = mp->next;
> @@ -197,14 +198,14 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
>
> np->next = mp;
> np->priority = skb_prio;
> - np->vlan_qos = vlan_qos;
> + np->vlan_prio = vlan_prio;
> /* Before inserting this element in hash table, make sure all its fields
> * are committed to memory.
> * coupled with smp_rmb() in vlan_dev_get_egress_qos_mask()
> */
> smp_wmb();
> vlan->egress_priority_map[skb_prio & 0xF] = np;
> - if (vlan_qos)
> + if (vlan_prio)
> vlan->nr_egress_mappings++;
> return 0;
> }
> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
> index cf5219df7903..f62d8320c5b4 100644
> --- a/net/8021q/vlan_netlink.c
> +++ b/net/8021q/vlan_netlink.c
> @@ -261,11 +261,11 @@ static int vlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
> for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
> for (pm = vlan->egress_priority_map[i]; pm;
> pm = pm->next) {
> - if (!pm->vlan_qos)
> + if (!pm->vlan_prio)
> continue;
>
> m.from = pm->priority;
> - m.to = (pm->vlan_qos >> 13) & 0x7;
> + m.to = pm->vlan_prio;
> if (nla_put(skb, IFLA_VLAN_QOS_MAPPING,
> sizeof(m), &m))
> goto nla_put_failure;
> diff --git a/net/8021q/vlanproc.c b/net/8021q/vlanproc.c
> index fa67374bda49..a5a5b8fbb054 100644
> --- a/net/8021q/vlanproc.c
> +++ b/net/8021q/vlanproc.c
> @@ -266,8 +266,8 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
> const struct vlan_priority_tci_mapping *mp
> = vlan->egress_priority_map[i];
> while (mp) {
> - seq_printf(seq, "%u:%d ",
> - mp->priority, ((mp->vlan_qos >> 13) & 0x7));
> + seq_printf(seq, "%u:%u ",
> + mp->priority, mp->vlan_prio);
> mp = mp->next;
> }
> }
> --
> 2.25.1
>
>
Powered by blists - more mailing lists