On Sun, May 22, 2022 at 12:42:04AM +0300, Vitaliy Makkoveev wrote:
> 'pipex_mppe' and 'pipex_session' structures have uint16_t bit fields
> which represent flags. We mix unlocked access to immutable flags with
> protected access to mutable ones. This could be not MP independent on
> some architectures, so convert them fields to u_int `flags' variables.
>
> bluhm@ pointed this when I have introduced `pxm_mtx' mutex(9),
> but the stack was not parallelized and all access was done with
> exclusive netlock. Now packet forwarding uses shared netlock and
> 'pipex_mppe' structure members could be accessed concurrently. So
> it's time to convert them.
OK bluhm@
> Index: sys/net/if_pppx.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.114
> diff -u -p -r1.114 if_pppx.c
> --- sys/net/if_pppx.c 22 Feb 2022 01:15:02 -0000 1.114
> +++ sys/net/if_pppx.c 21 May 2022 21:08:55 -0000
> @@ -1021,7 +1021,7 @@ pppacopen(dev_t dev, int flags, int mode
>
> /* virtual pipex_session entry for multicast */
> session = pool_get(&pipex_session_pool, PR_WAITOK | PR_ZERO);
> - session->is_multicast = 1;
> + session->flags |= PIPEX_SFLAGS_MULTICAST;
> session->ownersc = sc;
> sc->sc_multicast_session = session;
>
> Index: sys/net/pipex.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.136
> diff -u -p -r1.136 pipex.c
> --- sys/net/pipex.c 2 Jan 2022 22:36:04 -0000 1.136
> +++ sys/net/pipex.c 21 May 2022 21:08:55 -0000
> @@ -150,7 +150,7 @@ pipex_destroy_all_sessions(void *ownersc
> LIST_FOREACH_SAFE(session, &pipex_session_list, session_list,
> session_tmp) {
> if (session->ownersc == ownersc) {
> - KASSERT(session->is_pppx == 0);
> + KASSERT((session->flags & PIPEX_SFLAGS_PPPX) == 0);
> pipex_unlink_session(session);
> pipex_rele_session(session);
> }
> @@ -273,7 +273,7 @@ pipex_init_session(struct pipex_session
> session->ppp_flags = req->pr_ppp_flags;
> session->ppp_id = req->pr_ppp_id;
>
> - session->ip_forward = 1;
> + session->flags |= PIPEX_SFLAGS_IP_FORWARD;
>
> session->stat_counters = counters_alloc(pxc_ncounters);
>
> @@ -395,9 +395,9 @@ pipex_link_session(struct pipex_session
> session->ownersc = ownersc;
> session->ifindex = ifp->if_index;
> if (ifp->if_flags & IFF_POINTOPOINT)
> - session->is_pppx = 1;
> + session->flags |= PIPEX_SFLAGS_PPPX;
>
> - if (session->is_pppx == 0 &&
> + if ((session->flags & PIPEX_SFLAGS_PPPX) == 0 &&
> !in_nullhost(session->ip_address.sin_addr)) {
> if (pipex_lookup_by_ip_address(session->ip_address.sin_addr)
> != NULL)
> @@ -439,7 +439,7 @@ pipex_unlink_session(struct pipex_sessio
> NET_ASSERT_LOCKED();
> if (session->state == PIPEX_STATE_CLOSED)
> return;
> - if (session->is_pppx == 0 &&
> + if ((session->flags & PIPEX_SFLAGS_PPPX) == 0 &&
> !in_nullhost(session->ip_address.sin_addr)) {
> KASSERT(pipex_rd_head4 != NULL);
> rn = rn_delete(&session->ip_address, &session->ip_netmask,
> @@ -507,7 +507,11 @@ pipex_config_session(struct pipex_sessio
> return (EINVAL);
> if (session->ownersc != ownersc)
> return (EINVAL);
> - session->ip_forward = req->pcr_ip_forward;
> +
> + if (req->pcr_ip_forward != 0)
> + session->flags |= PIPEX_SFLAGS_IP_FORWARD;
> + else
> + session->flags &= ~PIPEX_SFLAGS_IP_FORWARD;
>
> return (0);
> }
> @@ -657,7 +661,7 @@ pipex_timer(void *ignored_arg)
> continue;
> /* Release the sessions when timeout */
> pipex_unlink_session(session);
> - KASSERTMSG(session->is_pppx == 0,
> + KASSERTMSG((session->flags & PIPEX_SFLAGS_PPPX) == 0,
> "FIXME session must not be released when pppx");
> pipex_rele_session(session);
> break;
> @@ -678,11 +682,12 @@ pipex_ip_output(struct mbuf *m0, struct
> {
> int is_idle;
>
> - if (session->is_multicast == 0) {
> + if ((session->flags & PIPEX_SFLAGS_MULTICAST) == 0) {
> /*
> * Multicast packet is a idle packet and it's not TCP.
> */
> - if (session->ip_forward == 0 && session->ip6_forward == 0)
> + if ((session->flags & (PIPEX_SFLAGS_IP_FORWARD |
> + PIPEX_SFLAGS_IP6_FORWARD)) == 0)
> goto drop;
> /* reset idle timer */
> if (session->timeout_sec != 0) {
> @@ -712,8 +717,8 @@ pipex_ip_output(struct mbuf *m0, struct
> LIST_FOREACH(session_tmp, &pipex_session_list, session_list) {
> if (session_tmp->ownersc != session->ownersc)
> continue;
> - if (session_tmp->ip_forward == 0 &&
> - session_tmp->ip6_forward == 0)
> + if ((session->flags & (PIPEX_SFLAGS_IP_FORWARD |
> + PIPEX_SFLAGS_IP6_FORWARD)) == 0)
> continue;
> m = m_copym(m0, 0, M_COPYALL, M_NOWAIT);
> if (m == NULL) {
> @@ -838,7 +843,7 @@ pipex_ppp_input(struct mbuf *m0, struct
>
> switch (proto) {
> case PPP_IP:
> - if (session->ip_forward == 0)
> + if ((session->flags & PIPEX_SFLAGS_IP_FORWARD) == 0)
> goto drop;
> if (!decrypted && pipex_session_is_mppe_required(session))
> /*
> @@ -850,7 +855,7 @@ pipex_ppp_input(struct mbuf *m0, struct
> return;
> #ifdef INET6
> case PPP_IPV6:
> - if (session->ip6_forward == 0)
> + if ((session->flags & PIPEX_SFLAGS_IP6_FORWARD) == 0)
> goto drop;
> if (!decrypted && pipex_session_is_mppe_required(session))
> /*
> @@ -2102,7 +2107,7 @@ pipex_mppe_init(struct pipex_mppe *mppe,
> memset(mppe, 0, sizeof(struct pipex_mppe));
> mtx_init(&mppe->pxm_mtx, IPL_SOFTNET);
> if (stateless)
> - mppe->stateless = 1;
> + mppe->flags |= PIPEX_MPPE_STATELESS;
> if (has_oldkey)
> mppe->old_session_keys =
> pool_get(&mppe_key_pool, PR_WAITOK);
> @@ -2273,7 +2278,7 @@ pipex_mppe_input(struct mbuf *m0, struct
> if (coher_cnt < mppe->coher_cnt)
> coher_cnt0 += 0x1000;
> if (coher_cnt0 - mppe->coher_cnt > 0x0f00) {
> - if (!mppe->stateless ||
> + if ((mppe->flags & PIPEX_MPPE_STATELESS) == 0 ||
> coher_cnt0 - mppe->coher_cnt
> <= 0x1000 - PIPEX_MPPE_NOLDKEY) {
> pipex_session_log(session, LOG_DEBUG,
> @@ -2286,7 +2291,7 @@ pipex_mppe_input(struct mbuf *m0, struct
> }
> }
>
> - if (mppe->stateless != 0) {
> + if ((mppe->flags & PIPEX_MPPE_STATELESS) != 0) {
> if (!rewind) {
> mppe_key_change(mppe);
> while (mppe->coher_cnt != coher_cnt) {
> @@ -2407,16 +2412,16 @@ pipex_mppe_output(struct mbuf *m0, struc
>
> mtx_enter(&mppe->pxm_mtx);
>
> - if (mppe->stateless != 0) {
> + if ((mppe->flags & PIPEX_MPPE_STATELESS) != 0) {
> flushed = 1;
> mppe_key_change(mppe);
> } else {
> if ((mppe->coher_cnt % 0x100) == 0xff) {
> flushed = 1;
> mppe_key_change(mppe);
> - } else if (mppe->resetreq != 0) {
> + } else if ((mppe->flags & PIPEX_MPPE_RESETREQ) != 0) {
> flushed = 1;
> - mppe->resetreq = 0;
> + mppe->flags &= ~PIPEX_MPPE_RESETREQ;
> }
> }
>
> @@ -2478,7 +2483,7 @@ pipex_ccp_input(struct mbuf *m0, struct
> case CCP_RESETREQ:
> PIPEX_DBG((session, LOG_DEBUG, "CCP RecvResetReq"));
> mtx_enter(&session->mppe_send.pxm_mtx);
> - session->mppe_send.resetreq = 1;
> + session->mppe_send.flags |= PIPEX_MPPE_RESETREQ;
> mtx_leave(&session->mppe_send.pxm_mtx);
> #ifndef PIPEX_NO_CCP_RESETACK
> PIPEX_DBG((session, LOG_DEBUG, "CCP SendResetAck"));
> Index: sys/net/pipex_local.h
> ===================================================================
> RCS file: /cvs/src/sys/net/pipex_local.h,v
> retrieving revision 1.45
> diff -u -p -r1.45 pipex_local.h
> --- sys/net/pipex_local.h 15 Feb 2022 03:31:17 -0000 1.45
> +++ sys/net/pipex_local.h 21 May 2022 21:08:55 -0000
> @@ -61,9 +61,10 @@
> /* mppe rc4 key */
> struct pipex_mppe {
> struct mutex pxm_mtx;
> - int16_t stateless:1, /* [I] key change mode */
> - resetreq:1, /* [m] */
> - reserved:14;
> + u_int flags; /* [m] flags, see below */
> +#define PIPEX_MPPE_STATELESS 0x01 /* [I] key change mode */
> +#define PIPEX_MPPE_RESETREQ 0x02 /* [m] */
> +
> int16_t keylenbits; /* [I] key length */
> int16_t keylen; /* [I] */
> uint16_t coher_cnt; /* [m] coherency counter */
> @@ -170,11 +171,15 @@ struct pipex_session {
> #define PIPEX_STATE_CLOSED 0x0004
>
> uint32_t idle_time; /* [N] idle time in seconds */
> - uint16_t ip_forward:1, /* [N] {en|dis}ableIP forwarding */
> - ip6_forward:1, /* [I] {en|dis}able IPv6 forwarding */
> - is_multicast:1, /* [I] virtual entry for multicast */
> - is_pppx:1, /* [I] interface is point2point(pppx) */
> - reserved:12;
> +
> + u_int flags; /* [N] flags, see below */
> +#define PIPEX_SFLAGS_IP_FORWARD 0x01 /* [N] enable IP
> forwarding */
> +#define PIPEX_SFLAGS_IP6_FORWARD 0x02 /* [N] enable IPv6 forwarding */
> +#define PIPEX_SFLAGS_MULTICAST 0x04 /* [I] virtual entry for
> + multicast */
> +#define PIPEX_SFLAGS_PPPX 0x08 /* [I] interface is
> + point2point(pppx) */
> +
> uint16_t protocol; /* [I] tunnel protocol (PK) */
> uint16_t session_id; /* [I] session-id (PK) */
> uint16_t peer_session_id; /* [I] peer's session-id */
