Re: About IFNET_PCP_NONE

From: Zhenlei Huang <zlei_at_FreeBSD.org>
Date: Tue, 29 Aug 2023 16:26:05 UTC

> On Aug 29, 2023, at 9:23 AM, Konstantin Belousov <kostikbel@gmail.com> wrote:
> 
> On Mon, Aug 28, 2023 at 10:25:01PM +0800, Zhenlei Huang wrote:
>> 
>> 
>>> On Aug 28, 2023, at 3:54 PM, Konstantin Belousov <kostikbel@gmail.com> wrote:
>>> 
>>> On Mon, Aug 28, 2023 at 09:51:38AM +0800, Zhenlei Huang wrote:
>>>> Hi Konstantin,
>>>> 
>>>> 
>>>> I was just about going to open a PR for https://reviews.freebsd.org/D39536 and
>>>> realized I might made wrong assumption.
>>>> 
>>>> I thought IFNET_PCP_NONE is something like IEEE8021Q_PCP_BE but I second why not
>>>> use IEEE8021Q_PCP_BE but a new const IFNET_PCP_NONE.
>> 
>> I meant
>> 
>> ```
>> int
>> ether_output_frame(struct ifnet *ifp, struct mbuf *m)
>> {
>>    uint8_t pcp;
>> 
>>    pcp = ifp->if_pcp;
>>    if (pcp != 0 /* IEEE8021Q_PCP_BE */ && ifp->if_type != IFT_L2VLAN &&
>>        !ether_set_pcp(&m, ifp, pcp))
>>        return (0);
>> 
>> ...
> This is wrong.  PCP_BE is just one of the priorities, that should allowed to
> be specified in the 802.1q pseudo-vlan header.
> 
>> 
>> }
>> ```
>> 
>>>> 
>>>> So despite its naming IFNET_PCP_NONE, is it actually a flag to let specific interface
>>>> completely bypass (disable) PCP processing?
>>>> 
>>>> The const IFNET_PCP_NONE is defined in sys/net/if.h with 
>>>> ```
>>>> #define IFNET_PCP_NONE 0xff   /* PCP disabled */
>>>> ```
>>> I fail to understand your question.
>>> 
>>> IFNET_PCP_NONE is a value that means that no 802.1q prio is inserted into
>>> the packet.  Otherwise, non-vlan traffic is tagged with the priority.
>> 
>> Think about the following case:
>> 
>> 1. Set interface's PCP to IFNET_PCP_NONE, application / firewall provide per-flow PCP, should
>> the traffic be tagged with the priority ?
> Yes, it should, but only for packets from the specified flows.

Thanks for the clarification !

I updated https://reviews.freebsd.org/D39536 <https://reviews.freebsd.org/D39536> as per discussion.

> 
>> 
>> 
>>> 
>>> IEEE8021Q_PCP_BE is a name of one of the priorities, it seems from my
>>> code reading.
>> 
>> 
>> Best regards,
>> Zhenlei
>>