> On 25 Jun 2020, at 20:13, Christiano F. Haesbaert <haesba...@haesbaert.org>
> wrote:
>
> You can.
>
> The idea is that ifindex is always monotonically increased, so to actually
> get a new interface you would have to have "overflowed" 65k interfaces,
> which is unreal.
>
> So if your interface is gone, you can be sure if_get will give you NULL.
No I can’t :) It’s *not* the operational infinity you talks about.
You can run:
while true ; do ifconfig tun0 create ; ifconfig tun0 destroy ; done
And after couple of minutes you will see is very real to have *counter* be
overflowed. Not the interface count.
>
> On Thu, Jun 25, 2020, 18:55 Vitaliy Makkoveev <henscheltig...@yahoo.com>
> wrote:
>
>> Updated diff.
>>
>> OpenBSD uses 16 bit counter for allocate interface indexes. So we can't
>> store index in session and be sure if_get(9) returned `ifnet' is our
>> original `ifnet'.
>>
>> Now each pipex(4) session has it's own reference to `ifnet'. Also pppoe
>> related sessions has the reference to it's ethernet interface.
>>
>> All `ifnet's are obtained by if_get(9) and we use `if_index' from stored
>> reference to `ifnet'.
>>
>> Index: net/if_pppx.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/if_pppx.c,v
>> retrieving revision 1.90
>> diff -u -p -r1.90 if_pppx.c
>> --- net/if_pppx.c 24 Jun 2020 08:52:53 -0000 1.90
>> +++ net/if_pppx.c 25 Jun 2020 16:41:25 -0000
>> @@ -714,15 +714,15 @@ pppx_add_session(struct pppx_dev *pxd, s
>> ifp->if_softc = pxi;
>> /* ifp->if_rdomain = req->pr_rdomain; */
>>
>> - error = pipex_link_session(session, &pxi->pxi_ifcontext);
>> - if (error)
>> - goto remove;
>> -
>> /* XXXSMP breaks atomicity */
>> NET_UNLOCK();
>> if_attach(ifp);
>> NET_LOCK();
>>
>> + error = pipex_link_session(session, &pxi->pxi_ifcontext);
>> + if (error)
>> + goto detach;
>> +
>> if_addgroup(ifp, "pppx");
>> if_alloc_sadl(ifp);
>>
>> @@ -771,7 +771,12 @@ pppx_add_session(struct pppx_dev *pxd, s
>>
>> return (error);
>>
>> -remove:
>> +detach:
>> + /* XXXSMP breaks atomicity */
>> + NET_UNLOCK();
>> + if_detach(ifp);
>> + NET_LOCK();
>> +
>> if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
>> panic("%s: inconsistent RB tree", __func__);
>> LIST_REMOVE(pxi, pxi_list);
>> @@ -860,13 +865,13 @@ pppx_if_destroy(struct pppx_dev *pxd, st
>> CLR(ifp->if_flags, IFF_RUNNING);
>>
>> pipex_unlink_session(session);
>> + pipex_rele_session(session);
>>
>> /* XXXSMP breaks atomicity */
>> NET_UNLOCK();
>> if_detach(ifp);
>> NET_LOCK();
>>
>> - pipex_rele_session(session);
>> if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
>> panic("%s: inconsistent RB tree", __func__);
>> LIST_REMOVE(pxi, pxi_list);
>> Index: net/pipex.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/pipex.c,v
>> retrieving revision 1.116
>> diff -u -p -r1.116 pipex.c
>> --- net/pipex.c 22 Jun 2020 09:38:15 -0000 1.116
>> +++ net/pipex.c 25 Jun 2020 16:41:25 -0000
>> @@ -148,7 +148,7 @@ pipex_iface_init(struct pipex_iface_cont
>> struct pipex_session *session;
>>
>> pipex_iface->pipexmode = 0;
>> - pipex_iface->ifnet_this = ifp;
>> + pipex_iface->ifnet_this = if_get(ifp->if_index);
>>
>> if (pipex_rd_head4 == NULL) {
>> if (!rn_inithead((void **)&pipex_rd_head4,
>> @@ -165,6 +165,7 @@ pipex_iface_init(struct pipex_iface_cont
>> session = pool_get(&pipex_session_pool, PR_WAITOK | PR_ZERO);
>> session->is_multicast = 1;
>> session->pipex_iface = pipex_iface;
>> + session->ifnet_this = if_get(ifp->if_index);
>> pipex_iface->multicast_session = session;
>> }
>>
>> @@ -194,10 +195,11 @@ pipex_iface_stop(struct pipex_iface_cont
>> void
>> pipex_iface_fini(struct pipex_iface_context *pipex_iface)
>> {
>> - pool_put(&pipex_session_pool, pipex_iface->multicast_session);
>> NET_LOCK();
>> pipex_iface_stop(pipex_iface);
>> NET_UNLOCK();
>> + pipex_rele_session(pipex_iface->multicast_session);
>> + if_put(pipex_iface->ifnet_this);
>> }
>>
>> int
>> @@ -358,7 +360,7 @@ pipex_init_session(struct pipex_session
>> MIN(req->pr_local_address.ss_len,
>> sizeof(session->local)));
>> #ifdef PIPEX_PPPOE
>> if (req->pr_protocol == PIPEX_PROTO_PPPOE)
>> - session->proto.pppoe.over_ifidx = over_ifp->if_index;
>> + session->proto.pppoe.over_iface =
>> if_get(over_ifp->if_index);
>> #endif
>> #ifdef PIPEX_PPTP
>> if (req->pr_protocol == PIPEX_PROTO_PPTP) {
>> @@ -421,6 +423,13 @@ pipex_init_session(struct pipex_session
>> void
>> pipex_rele_session(struct pipex_session *session)
>> {
>> +#ifdef PIPEX_PPPOE
>> + if (session->protocol == PIPEX_PROTO_PPPOE)
>> + if_put(session->proto.pppoe.over_iface);
>> +#endif
>> + if (session->ifnet_this) {
>> + if_put(session->ifnet_this);
>> + }
>> if (session->mppe_recv.old_session_keys)
>> pool_put(&mppe_key_pool,
>> session->mppe_recv.old_session_keys);
>> pool_put(&pipex_session_pool, session);
>> @@ -439,6 +448,7 @@ pipex_link_session(struct pipex_session
>> return (EEXIST);
>>
>> session->pipex_iface = iface;
>> + session->ifnet_this = if_get(iface->ifnet_this->if_index);
>>
>> LIST_INSERT_HEAD(&pipex_session_list, session, session_list);
>> chain = PIPEX_ID_HASHTABLE(session->session_id);
>> @@ -655,7 +665,7 @@ pipex_destroy_session(struct pipex_sessi
>> }
>>
>> pipex_unlink_session(session);
>> - pool_put(&pipex_session_pool, session);
>> + pipex_rele_session(session);
>>
>> return (0);
>> }
>> @@ -919,7 +929,7 @@ pipex_ip_output(struct mbuf *m0, struct
>> struct ifnet *ifp;
>>
>> /* output succeed here as a interface */
>> - ifp = session->pipex_iface->ifnet_this;
>> + ifp = session->ifnet_this;
>> ifp->if_opackets++;
>> ifp->if_obytes+=m0->m_pkthdr.len;
>>
>> @@ -1038,7 +1048,7 @@ pipex_ppp_input(struct mbuf *m0, struct
>>
>> #if NBPFILTER > 0
>> {
>> - struct ifnet *ifp = session->pipex_iface->ifnet_this;
>> + struct ifnet *ifp = session->ifnet_this;
>> if (ifp->if_bpf && ifp->if_type == IFT_PPP)
>> bpf_mtap(ifp->if_bpf, m0, BPF_DIRECTION_IN);
>> }
>> @@ -1105,7 +1115,7 @@ pipex_ip_input(struct mbuf *m0, struct p
>> int is_idle;
>>
>> /* change recvif */
>> - ifp = session->pipex_iface->ifnet_this;
>> + ifp = session->ifnet_this;
>> m0->m_pkthdr.ph_ifidx = ifp->if_index;
>>
>> if (ISSET(session->ppp_flags, PIPEX_PPP_INGRESS_FILTER)) {
>> @@ -1174,7 +1184,7 @@ pipex_ip6_input(struct mbuf *m0, struct
>> int len;
>>
>> /* change recvif */
>> - ifp = session->pipex_iface->ifnet_this;
>> + ifp = session->ifnet_this;
>> m0->m_pkthdr.ph_ifidx = ifp->if_index;
>>
>> /*
>> @@ -1346,7 +1356,8 @@ pipex_pppoe_lookup_session(struct mbuf *
>> PIPEX_DBG((NULL, LOG_DEBUG, "<%s> session not found
>> (id=%d)",
>> __func__, pppoe.session_id));
>> #endif
>> - if (session && session->proto.pppoe.over_ifidx !=
>> m0->m_pkthdr.ph_ifidx)
>> + if (session && session->proto.pppoe.over_iface->if_index !=
>> + m0->m_pkthdr.ph_ifidx)
>> session = NULL;
>>
>> return (session);
>> @@ -1407,19 +1418,12 @@ pipex_pppoe_output(struct mbuf *m0, stru
>> pppoe->session_id = htons(session->session_id);
>> pppoe->length = htons(len);
>>
>> - m0->m_pkthdr.ph_ifidx = session->proto.pppoe.over_ifidx;
>> + ifp = session->proto.pppoe.over_iface;
>> + m0->m_pkthdr.ph_ifidx = ifp->if_index;
>> m0->m_flags &= ~(M_BCAST|M_MCAST);
>> -
>> - ifp = if_get(session->proto.pppoe.over_ifidx);
>> - if (ifp != NULL) {
>> - ifp->if_output(ifp, m0, &session->peer.sa, NULL);
>> - session->stat.opackets++;
>> - session->stat.obytes += len;
>> - } else {
>> - m_freem(m0);
>> - session->stat.oerrors++;
>> - }
>> - if_put(ifp);
>> + ifp->if_output(ifp, m0, &session->peer.sa, NULL);
>> + session->stat.opackets++;
>> + session->stat.obytes += len;
>> }
>> #endif /* PIPEX_PPPOE */
>>
>> Index: net/pipex_local.h
>> ===================================================================
>> RCS file: /cvs/src/sys/net/pipex_local.h,v
>> retrieving revision 1.35
>> diff -u -p -r1.35 pipex_local.h
>> --- net/pipex_local.h 18 Jun 2020 14:20:12 -0000 1.35
>> +++ net/pipex_local.h 25 Jun 2020 16:41:25 -0000
>> @@ -78,7 +78,7 @@ struct pipex_mppe {
>>
>> #ifdef PIPEX_PPPOE
>> struct pipex_pppoe_session {
>> - u_int over_ifidx; /* [I] ether interface */
>> + struct ifnet *over_iface; /* [I] ether interface */
>> };
>> #endif /* PIPEX_PPPOE */
>>
>> @@ -183,6 +183,7 @@ struct pipex_session {
>> int ip6_prefixlen; /* [I] remote IPv6 prefixlen */
>>
>> struct pipex_iface_context* pipex_iface; /* [I] context for
>> interface */
>> + struct ifnet *ifnet_this; /* [k] outer interface */
>>
>> uint32_t ppp_flags; /* [I] configure flags */
>> #ifdef PIPEX_MPPE
>>
