Re: [PATCH] PPPoL2TP: Add more code snippets

Guillaume Nault
2023-04-18T13:38:54Z

On Tue, Apr 18, 2023 at 01:54:09PM +0200, Samuel Thibault wrote:
> Guillaume Nault, le mar. 18 avril 2023 13:25:38 +0200, a ecrit:
> > As I said in my previous reply, a simple L2TP example that goes until PPP
> > channel and unit creation is fine. But any more advanced use of the PPP
> > API should be documented in the PPP documentation.
>
> When it's really advanced, yes. But here it's just about tunnel
> bridging, which is a very common L2TP thing to do.

I can't undestand why you absolutely want this covered in l2tp.rst.
This feature also works on PPPoE.

Also, it's probably a desirable feature, but certainly not a common
thing on Linux. This interface was added a bit more than 2 years ago,
which is really recent considering the age of the code. Appart from
maybe go-l2tp, I don't know of any user.

> > I mean, these files document the API of their corresponding modules,
> > their scope should be limitted to that (the PPP and L2TP layers are
> > really different).
>
> I wouldn't call
>
> + ret = ioctl(ppp_chan_fd, PPPIOCBRIDGECHAN, &chindx2);
> + close(ppp_chan_fd);
> + if (ret < 0)
> + return -errno;
>
> documentation...

The documentation is in ppp_generic.rst. Does it really make sense to
you to have the doc there and the sample code in l2tp.rst?

Anyway, I'm not going to argue any longer. You have my opinion.
You're always free to ignore feedbacks.

> > That shouldn't preclude anyone from describing how to combine L2TP, PPP
> > and others to cover more advanced use cases. It's just better done in a
> > different file.
>
> A more complete example, yes. I don't plan on taking time to do it.
>
> Samuel
>