Received-SPF: pass (google.com: domain of linux-wireless+bounces-877-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: "Peer, Ilan" <ilan.peer@intel.com>
To: Jouni Malinen <j@w1.fi>, "Korenblit, Miriam Rachel"
	<miriam.rachel.korenblit@intel.com>
CC: "johannes@sipsolutions.net" <johannes@sipsolutions.net>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>, "Greenman,
 Gregory" <gregory.greenman@intel.com>, "Berg, Johannes"
	<johannes.berg@intel.com>
Subject: RE: [PATCH 06/14] wifi: cfg80211: Update the default DSCP-to-UP
 mapping
Thread-Topic: [PATCH 06/14] wifi: cfg80211: Update the default DSCP-to-UP
 mapping
Thread-Index: AQHaK2mRrxuL2hBpb0mDmxS+sJxsd7CtQ8OAgAAoClA=
Date: Sun, 17 Dec 2023 15:11:18 +0000
Message-ID: <DM4PR11MB60430BF89551B1935E4472A0E991A@DM4PR11MB6043.namprd11.prod.outlook.com>
References: <20231211070532.2458539-1-miriam.rachel.korenblit@intel.com>
 <20231211085121.8a1c7d1f0034.I50aed38be78ae9aea052938e2cb6b5800010ecd4@changeid>
 <ZX7CbSCEv7Zvv476@w1.fi>
In-Reply-To: <ZX7CbSCEv7Zvv476@w1.fi>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?l/kFR1tO7qNeE/4XG2YL4eLLnmqcLp4Ea8czgIzyphaksEJOXwUCZbRTIsrE?=
 =?us-ascii?Q?UJeuFwvuB3VeavUn92kUyObrhXxffpnowLzxCRsj0l50OOaNsB54r2uas8eg?=
 =?us-ascii?Q?jsCp0h1jaIHsRv5ppgDchc0A4BCOKEHPPHmTJrSpVOyIQie9lY9nEG1Q3leW?=
 =?us-ascii?Q?sPLZnfFeXUAwrusG8itfaaKC++MvTmUwVdlrA5FdZFP5QORzQmlVjWQJ7Mvm?=
 =?us-ascii?Q?lfHSdNSItYRGBaIkroy94QlkOS1E8DxVU1UoW00w8J29IDfNfJfMu7pI4R2B?=
 =?us-ascii?Q?u6Qaujo/Zlu7nsgRU2zcGfdTDocSlZqfTBNziwrGdUOIjHB8ibHybXC0Wqtr?=
 =?us-ascii?Q?9OdO8zeLWM/qUAjUg9XSD5n7TvP0OEfFA4EZiHuJnVfFYBhrDq8LFRgtXJHD?=
 =?us-ascii?Q?aPNN177J38zQzQ/XcwsB8LHuhBqPTbhdFXFW20IDC9s9jodW5/uaBnSSRvwt?=
 =?us-ascii?Q?CqMnATqMsi0e4Y0+0R0vVM96T4zs02TfMnY2Pdx57sbXfT1B/LMDoEalwVtS?=
 =?us-ascii?Q?4XpVHZjPUJgu3c6ul6bDyo3J/TED3CcuQYHh16llnT1dpwDDUebdngt6ihI9?=
 =?us-ascii?Q?NYYogdG7lmQuPfJCPgfTh51/qg3qRpMKDfmzpOpGWz3xp6sDPSoA5saLlfaN?=
 =?us-ascii?Q?IPfAGKIttNKHyhj9ovvYLFdHXRg0E1Ig/cF68CtaMPpCS/cE2zXF85KCD8TJ?=
 =?us-ascii?Q?pCQTSLpQAyVo/ZrpYeXjy7D9XCV5Fl+T500sQoUuj/VIRzi/wONZD5vay3D2?=
 =?us-ascii?Q?o5K/lEFZxank0Oxd6shyUULSM8G2HgDf/kanJW3Qamq56tBtkQZLdu8jjBbw?=
 =?us-ascii?Q?5nkD3XVv8RJV6rFYAz0kI8pbV3EyRZayMjEGanL9GoN2Z7F68cN5GYU3aAXI?=
 =?us-ascii?Q?PLvKmBTYwdIRqIF5KDkLWdyajhJxRblVhHEtzpJuiemAbfEfZ1QLxr9C1aKX?=
 =?us-ascii?Q?EaDjQcwYd7EjkN8PWOUlr5OnmT9tzthqZhk++LiVtyxQXXqyLCRIRhH6lz6D?=
 =?us-ascii?Q?ce2iNG/V2oW/Pw0eYvmMmwDdJJ7kYCrJI3fXRG77LHLqO1dEy+Ykp7zXhZkY?=
 =?us-ascii?Q?dE8jljWydHSMEvCfzQV33c0sxhRIon6I8N87tAwtluoNhBUmGll4+zGxhfsH?=
 =?us-ascii?Q?yCAgbnSL9w5eCwzt8BDGDmuzOuqGBKDHelfWlMj4HTj0fK9blHWkG7YAti2k?=
 =?us-ascii?Q?ECx8l9h2O4vRR0nSGQQKgEvt6txV7V6NxbTDyXOH3PUWa+x3sN3pxzl07rB5?=
 =?us-ascii?Q?7b1SldwaZuwDmZ5sXvW0Ijav10jShiUNecbRWTCfdyMrxZlNWS2haE5+N+ZA?=
 =?us-ascii?Q?VZ8d91hOV0zdITYFBd5LVa/ubYicTJ6VVujD6gpUCtBwfGDSSiIfck45Lkxc?=
 =?us-ascii?Q?iFfIjGt7M/jHUWfN76bjcKxSgXfNKYkNw05Gt2divPHJ92ribU/qUxyyj0qU?=
 =?us-ascii?Q?GZGUqP7ERHW9mdba1zEBp1DJPmd3z3LQ7Enhd/NFLMtUrxZUrbIhekLYJEDH?=
 =?us-ascii?Q?kUAIsHrHbBp52RymTXYXoenME8R/CLm4efJfW/1fyWlVE30mCEog61x6dnC5?=
 =?us-ascii?Q?3pPd/q+3PwadvHMdoPEyuIpFysmw7Fd4YtnB0RLq?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-wireless@vger.kernel.org
List-Id: <linux-wireless.vger.kernel.org>
List-Subscribe: <mailto:linux-wireless+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-wireless+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6043.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: df806e4e-c04c-4635-c39d-08dbff126bdd
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Dec 2023 15:11:18.6812
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: wQ3n0aKIGTnyjX0VVx0Gc2fatq+eBmhqk4iqX3xB0lzgPhWwLDmZiBRwlaCCKWXKxMRGKqkT/jYn0XaUddYESA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB8124
X-OriginatorOrg: intel.com

Hi,

> -----Original Message-----
> From: Jouni Malinen <j@w1.fi>
> Sent: Sunday, December 17, 2023 11:42
> To: Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>
> Cc: johannes@sipsolutions.net; linux-wireless@vger.kernel.org; Peer, Ilan
> <ilan.peer@intel.com>; Greenman, Gregory <gregory.greenman@intel.com>;
> Berg, Johannes <johannes.berg@intel.com>
> Subject: Re: [PATCH 06/14] wifi: cfg80211: Update the default DSCP-to-UP
> mapping
>=20
> On Mon, Dec 11, 2023 at 09:05:24AM +0200, Miri Korenblit wrote:
> > The default DSCP-to-UP mapping method defined in RFC8325 applied to
> > packets marked per recommendations in RFC4594 and destined to 802.11
> > WLAN clients will yield a number of inconsistent QoS mappings.
> >
> > To handle this, modify the mapping of specific DSCP values for which
> > the default mapping will create inconsistencies, based on the
> > recommendations in section 4 in RFC8325.
>=20
> Could this commit message give some more justification for why these exac=
t
> specifications are the best source of this information? For example, shou=
ld this
> also reference the Wi-Fi QoS Management specification?
>=20

I was using these specifications since they were mentioned in the Wi-Fi QoS=
 Management specification
and because RFC8325 was specifically mentioned in the WFA test plan specifi=
cations.
I'll update the commit message.=20

> > diff --git a/net/wireless/util.c b/net/wireless/util.c @@ -980,7
> > +980,53 @@ unsigned int cfg80211_classify8021d(struct sk_buff *skb,
> >  		}
> >  	}
> >
> > +	/* The default mapping as defined in RFC8325 */
> >  	ret =3D dscp >> 5;
>=20
> Could you point to the exact location in RFC 8325 that has that as the de=
fault
> mapping? Figure 1 has this note: "Note: All unused codepoints are
> RECOMMENDED to be mapped to UP 0".
>=20
> Section 2.4 does describe what many APs do, but that section is called
> "Default UP-to-DSCP Mappings and Conflicts" which does not sound like a
> recommended default mapping in RFC 8325..
>=20

The term "Default DSCP-to-UP" is defined in section 2.3 in RFC8325. As note=
d there, this is
not a specification definition, but more a term to describe what is a "comm=
on practice in the
networking industry".

> > +	/* Handle specific DSCP values for which the default mapping doesn't
> > +	 * adhere to the intended usage of the DSCP value. See section 4 in
> > +	 * RFC8325.
> > +	 */
> > +	switch (dscp >> 2) {
>=20
> What about "Standard: DF" (0 --> 0) and "Low-Priority Data: CS1" (8 -> 1)=
?
> Are those omitted here because the "dscp >> 5" happens to have same value=
?
> It would seem to be good to have at least a comment here pointing that ou=
t in
> particular taken into account that comment above about the status of that
> "default mapping" and how it relates to RFC 8325.
>=20

Yes. I thought my comment above was clear about it, but I'll rephrase it to=
 make
It clearer.

> > +	case 10:
> > +	case 12:
> > +	case 14:
> > +		/* High throughput data: AF11, AF12, AF13 */
> > +		ret =3D 0;
> > +		break;
> > +	case 16:
> > +		/* Operations, Administration, and Maintenance and
> Provisioning:
> > +		 * CS2
> > +		 */
> > +		ret =3D 0;
> > +		break;
> > +	case 18:
> > +	case 20:
> > +	case 22:
> > +		/* Low latency data: AF21, AF22, AF23 */
> > +		ret =3D 3;
> > +		break;
> > +	case 24:
> > +		/* Broadcasting video: CS23 */
> > +		ret =3D 4;
> > +		break;
>=20
> Typo.. Should be "CS3" instead of "CS23".
>=20

Fixed.

> What about "Multimedia Streaming: AF31, AF32, AF33"? Shouldn't those
> values 26, 28, 30 be mapped to 4? If not, it would be good to add a comme=
nt
> explaining why that is not here while it is included in RFC 8325 Figure 1=
.
>=20

The default mapping maps AF31, AF32 and AF33 to 4 which is the recommended =
mapping.
I'll add a comment.

> What about "Real-Time Interactive: CS4"? Shouldn't that value 32 be mappe=
d
> to 4? If not, it would be good to add a comment explaining why that is no=
t
> here while it is included in RFC 8325 Figure 1.
>=20

Same here. The default mapping is used. I'll add a comment.

> What about "Multimedia Conferencing: AF41, AF42, AF43"? Shouldn't those
> values 34, 36, 38 be mapped to 4? If not, it would be good to add a comme=
nt
> explaining why that is not here while it is included in RFC 8325 Figure 1=
.
>=20

Same.

> > +	case 40:
> > +		/* Signaling: CS5 */
> > +		ret =3D 5;
> > +		break;
> > +	case 44:
> > +		/* Voice Admit */
> > +		ret =3D 6;
> > +		break;
>=20
> To be consistent, that comment should be "VOICE-ADMIT: VA".
>=20

Fixed.

> > +	case 46:
> > +		/* Telephony traffic: EF */
> > +		ret =3D 6;
> > +		break;
> > +	case 48:
> > +		/* Network Control Traffic: CS6 */
> > +		ret =3D 7;
> > +		break;
> > +	}
>=20
> This does not include "Network Control: CS7". Is there a reason for not
> covering that case 56 now? I know it is "reserved for future use", but RF=
C
> 8325 does provide mapping for it as well.
>=20
=20
Same.

Regards,

Ilan.