Re: [PATCH net v4 1/2] flow_dissector: do not dissect PPPoE PFC frames

[Qingfang Deng]

Hi,

On 4/11/2026 1:10 AM, Simon Horman wrote:
> On Fri, Apr 10, 2026 at 11:36:20AM +0800, Qingfang Deng wrote:
> > @@ -1361,7 +1376,7 @@ bool __skb_flow_dissect(const struct net *net,
> >   			struct pppoe_hdr hdr;
> >   			__be16 proto;
> >   		} *hdr, _hdr;
> > -		u16 ppp_proto;
> > +		__be16 ppp_proto;
>
> I'm unclear of the relationship between changing the type of ppp_proto
> and the problem described in the patch description. And it
> is creating a log of churn in this patch. I suggest dropping it.

The intention is to restore the original behavior before the blamed 
commit. If you find it too verbose for a fix, I can drop it and then 
repost that part later to net-next.

> > @@ -1374,27 +1389,19 @@ bool __skb_flow_dissect(const struct net *net,
> >   			break;
> >   		}
> >   
> > -		/* least significant bit of the most significant octet
> > -		 * indicates if protocol field was compressed
> > -		 */
> > -		ppp_proto = ntohs(hdr->proto);
> > -		if (ppp_proto & 0x0100) {
> > -			ppp_proto = ppp_proto >> 8;
> > -			nhoff += PPPOE_SES_HLEN - 1;
> > -		} else {
> > -			nhoff += PPPOE_SES_HLEN;
> > -		}
>
> Could we go for something like this?
>
> 		ppp_proto = ntohs(hdr->proto);
> 		nhoff += PPPOE_SES_HLEN;
>
> 		/* Explanation of what is going on */
> 		if (ppp_proto & 0x0100)
> 			ppp_proto = some invalid value like 0

I think it is redundant. ppp_proto_is_valid() already requires 
uncompressed frames.

> > +		ppp_proto = hdr->proto;
> > +		nhoff += PPPOE_SES_HLEN;
> >   
> > -		if (ppp_proto == PPP_IP) {
> > +		if (ppp_proto == htons(PPP_IP)) {
> >   			proto = htons(ETH_P_IP);
> >   			fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > -		} else if (ppp_proto == PPP_IPV6) {
> > +		} else if (ppp_proto == htons(PPP_IPV6)) {
> >   			proto = htons(ETH_P_IPV6);
> >   			fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > -		} else if (ppp_proto == PPP_MPLS_UC) {
> > +		} else if (ppp_proto == htons(PPP_MPLS_UC)) {
> >   			proto = htons(ETH_P_MPLS_UC);
> >   			fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> > -		} else if (ppp_proto == PPP_MPLS_MC) {
> > +		} else if (ppp_proto == htons(PPP_MPLS_MC)) {
> >   			proto = htons(ETH_P_MPLS_MC);
> >   			fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> >   		} else if (ppp_proto_is_valid(ppp_proto)) {