On Fri, Jul 15, 2022 at 02:05:12PM +0300, Vitaliy Makkoveev wrote:
> I see no reason to move 'not_ours' checks outside pipex_common_input()
> because this introduce code duplication. Also I see no reason to take
> session lock after pipex_ppp_input() call, because it will be released
> by caller just after successful return. At least fdrelease() has the
> same behavior.
This is better. Can we get symmetric mutex enter and leave by
splitting pipex_common_input() in two funtions? The first part
needs the lock and checks whether the packet is ours.
Maybe something like this:
mtx_enter(&session->pxs_mtx);
...
if (pipex_isours()) {
mtx_leave(&session->pxs_mtx);
pipex_common_input();
return (NULL);
}
...
mtx_leave(&session->pxs_mtx);
We could also commit the current code as it is with the locked
paramter and improve in tree.
> @@ -1261,8 +1275,8 @@ pipex_pppoe_input(struct mbuf *m0, struc
> sizeof(struct pipex_pppoe_header), &pppoe);
>
> hlen = sizeof(struct ether_header) + sizeof(struct pipex_pppoe_header);
> - if ((m0 = pipex_common_input(session, m0, hlen, ntohs(pppoe.length)))
> - == NULL)
> + m0 = pipex_common_input(session, m0, hlen, ntohs(pppoe.length), 0);
> + if (m0 != NULL)
> return (NULL);
> m_freem(m0);
Is this the wrong way around? Should be if (m0 == NULL) return (NULL);
bluhm
