Re: git: 28903f396af4 - main - ng_pppoe: introduce new sysctl net.graph.pppoe.lcp_pcp

From: Gleb Smirnoff <glebius_at_freebsd.org>
Date: Mon, 02 May 2022 20:53:31 UTC
  Eugene,

On Mon, May 02, 2022 at 02:57:19PM +0000, Eugene Grosbein wrote:
E> commit 28903f396af4b151e16ea606cda66a9244fb179f
E> Author:     Eugene Grosbein <eugen@FreeBSD.org>
E> AuthorDate: 2022-05-02 14:55:24 +0000
E> Commit:     Eugene Grosbein <eugen@FreeBSD.org>
E> CommitDate: 2022-05-02 14:57:12 +0000
E> 
E>     ng_pppoe: introduce new sysctl net.graph.pppoe.lcp_pcp
E>     
E>     This fixes incomplete commit 2e547442ab3822d3d7c46a68f152032ef5fe337c
E>     
E>     New sysctl allows to mark transmitted PPPoE LCP Control
E>     ethernet frames with needed 3-bit Priority Code Point (PCP) value.
E>     Confirming driver like if_vlan(4) uses the value to fill
E>     IEEE 802.1p class of service field.
E>     
E>     This is similar to Cisco IOS "control-packets vlan cos priority"
E>     command.
E>     
E>     It helps to avoid premature disconnection of user sessions
E>     due to control frame drops (LCP Echo etc.)
E>     if network infrastructure has a botteleck at a switch
E>     or the xdsl DSLAM.
E>     
E>     See also:
E>     https://sourceforge.net/p/mpd/discussion/44692/thread/c7abe70e3a/
E>     
E>     Tested by:      Klaus Fokuhl at SourceForge
E>     MFC after:      2 weeks
E> ---
E>  share/man/man4/ng_pppoe.4 |  2 +-
E>  sys/netgraph/ng_pppoe.c   | 22 ++++++++++++++++++++++
E>  2 files changed, 23 insertions(+), 1 deletion(-)
E> 
E> diff --git a/share/man/man4/ng_pppoe.4 b/share/man/man4/ng_pppoe.4
E> index d9853a746512..ff53d4ef3a95 100644
E> --- a/share/man/man4/ng_pppoe.4
E> +++ b/share/man/man4/ng_pppoe.4
E> @@ -322,7 +322,7 @@ control message, when all session have been disconnected or when the
E>  hook is disconnected.
E>  .Sh SYSCTL VARIABLES
E>  The node can mark transmitted LCP Ethernet packets (protocol 0xc021)
E> -with 3-bit Priority code point (PCP) referring to IEEE 802.1p
E> +with 3-bit Priority Code Point (PCP) referring to IEEE 802.1p
E>  class of service with following
E>  .Xr sysctl 8
E>  variable.
E> diff --git a/sys/netgraph/ng_pppoe.c b/sys/netgraph/ng_pppoe.c
E> index 8bc44e160044..5a327f95b930 100644
E> --- a/sys/netgraph/ng_pppoe.c
E> +++ b/sys/netgraph/ng_pppoe.c
E> @@ -49,8 +49,13 @@
E>  #include <sys/malloc.h>
E>  #include <sys/errno.h>
E>  #include <sys/epoch.h>
E> +#include <sys/socket.h>
E> +#include <sys/sysctl.h>
E>  #include <sys/syslog.h>
E>  #include <net/ethernet.h>
E> +#include <net/if.h>
E> +#include <net/if_vlan_var.h>
E> +#include <net/vnet.h>
E>  
E>  #include <netgraph/ng_message.h>
E>  #include <netgraph/netgraph.h>
E> @@ -64,8 +69,19 @@ static MALLOC_DEFINE(M_NETGRAPH_PPPOE, "netgraph_pppoe", "netgraph pppoe node");
E>  #define M_NETGRAPH_PPPOE M_NETGRAPH
E>  #endif
E>  
E> +/* Some PPP protocol numbers we're interested in */
E> +#define PROT_LCP		0xc021
E> +
E>  #define SIGNOFF "session closed"
E>  
E> +VNET_DEFINE_STATIC(u_int32_t, ng_pppoe_lcp_pcp) = 0;
E> +#define V_ng_pppoe_lcp_pcp	VNET(ng_pppoe_lcp_pcp)
E> +
E> +SYSCTL_NODE(_net_graph, OID_AUTO, pppoe, CTLFLAG_RW, 0, "PPPoE");
E> +SYSCTL_UINT(_net_graph_pppoe, OID_AUTO, lcp_pcp,
E> +	CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(ng_pppoe_lcp_pcp), 0,
E> +	"Set PCP for LCP");
E> +
E>  /*
E>   * This section contains the netgraph method declarations for the
E>   * pppoe node. These methods define the netgraph pppoe 'type'.
E> @@ -1438,6 +1454,12 @@ ng_pppoe_rcvdata(hook_p hook, item_p item)
E>  			    mtod(m, u_char *)[1] == 0x03)
E>  				m_adj(m, 2);
E>  		}
E> +
E> +		if (V_ng_pppoe_lcp_pcp && m->m_pkthdr.len >= 2 &&
E> +		    m->m_len >= 2 && (m = m_pullup(m, 2)) &&
E> +		    mtod(m, uint16_t *)[0] == htons(PROT_LCP))
E> +			EVL_APPLY_PRI(m, (uint8_t)(V_ng_pppoe_lcp_pcp & 0x7));
E> +
E>  		/*
E>  		 * Bang in a pre-made header, and set the length up
E>  		 * to be correct. Then send it to the ethernet driver.

So some packets sent by ng_ppp(4) need to be specially tagged so that vlan(4)
understands them. Why do we make this tagging in ng_pppoe(4) rather than in
ng_ppp(4)?

-- 
Gleb Smirnoff