[PATCH][RFC] udhcp: add option to set CoS priority

[Clément Péron]

Hi Bernhard,

On Fri, 13 Jan 2023 at 21:08, Bernhard Reutner-Fischer
<rep.dot.nop at gmail.com> wrote:
>
> On Fri, 13 Jan 2023 10:55:26 +0100
> Clément Péron <peron.clem at gmail.com> wrote:
>
> > Hi,
> >
> >
> > On Fri, 13 Jan 2023 at 10:48, Clément Péron <peron.clem at gmail.com> wrote:
> > >
> > > Some ISP, like the French ISP Orange uses DHCP messages with
> > > a CoS Priority of 6 otherwise they are not processed.
> > >
> > > Add an option to allow setting this property.
> >
> > Please note, that I get this information from this blog post
> > https://www.lafois.com/tag/udhcp/
> >
> > I'm still testing this patch and I'm unsure if we need to set the
> > priority for all the sockets.
> >
> > I recovered a patch from Ubiquiti GPL archive where only
> >
> > udhcp_send_raw_packet() set the priority and not udhcp_send_kernel_packet().
> >
> > I'm not sure which one is correct.
>
> I admit that i did not look, so cannot comment.
>
> >
> > Thanks for your help,
> > BR,
> > Clement
>
> > > diff --git a/networking/udhcp/d6_packet.c b/networking/udhcp/d6_packet.c
> > > index 142de9b43..425037ada 100644
> > > --- a/networking/udhcp/d6_packet.c
> > > +++ b/networking/udhcp/d6_packet.c
> > > @@ -68,6 +68,13 @@ int FAST_FUNC d6_send_raw_packet_from_client_data_ifindex(
> > >                 goto ret_msg;
> > >         }
> > >
> > > +       IF_FEATURE_UDHCPC_COS(
> > > +       if (sk_prio) {
> > > +               if (setsockopt_int(fd, SOL_SOCKET, SO_PRIORITY, sk_prio)) {
>
> setsockopt_SOL_SOCKET_int() ?
>
> > > +                       log1s("raw: SO_PRIORITY setsockopt() failed");
> > > +               }
>
> Maybe add a common helper to udhcp like
> setsockopt_priority(sk_prio) that does
> setsockopt_SOL_SOCKET_int() || log1s()

Good point,
I will send a v2 with a helper function in common.c
void udhcp_socket_prio(int fd);

Also I will move the global int sk_prio; in common.c and declare it in
common.h which is more logic than packet.c

> since you seem to do that more than once?
>
> > > +//usage:       IF_FEATURE_UDHCPC_COS(
> > > +//usage:     "\n       -y PRIORITY     CoS value 0 .. 7, default 0"
>
> I don't see that you would cap the value to 7 anywhere, do you?
> The manpage seems to imply that 0..6 can be used by unprivileged users,
> higher values require CAP_NET_ADMIN which is fine per se; I assume the
> kernel does enough sanity-checking so we can attempt to pass whatever
> the user said.
> thanks,

Agree will remove this in the usage documentation.

Thanks for your review!