On Tue, Jul 20, 2021 at 02:25:34AM +0300, Vitaliy Makkoveev wrote:
> With bluhm@'s "forwarding in parallel" diff pipex(4) session could be
> accessed in parallel through (*ifp->if_input) -> ether_input().
>
> Except statistics and MPPE data PPPOE sessions are mostly immutable. We
> have only place where we should reset 'idle_time' in input path with
> shared netlock. But since pipex(4) garbage collector uses exclusive
> netlock we could reset it with shared netlock.
>
> At the first step I propose to turn session statistics to per-CPU
> counters. This make sense because we could have sessions with MPPE
> disabled and PPPOE input will be lockless for this case.
OK bluhm@ with one nit
> +void
> +pipex_export_session_stats(struct pipex_session *session,
> + struct pipex_statistics *stats)
> +{
> + uint64_t counters[pxc_ncounters];
> +
> + counters_read(session->stat_counters, counters, pxc_ncounters);
> +
Could you put a memset(stats, 0, sizeof(struct pipex_statistics))
here? When copying to userland we have to be extra careful. On
some architectures padding may be different. Structs may change.
With memset() we cannot leak kernel memory.
> + stats->ipackets = counters[pxc_ipackets];
> + stats->ierrors = counters[pxc_ierrors];
> + stats->ibytes = counters[pxc_ibytes];
> + stats->opackets = counters[pxc_opackets];
> + stats->oerrors = counters[pxc_oerrors];
> + stats->obytes = counters[pxc_obytes];
> + stats->idle_time = session->idle_time;
> +}
