<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body>
<p>Hi Eduardo,</p>
<p>can you rebase it to master branch?</p>
<p>Do you have any readme or summary, how that feature would be
used?</p>
<p>My first note is it adds HAVE_UBUS define to lease structure, but
it seems more or less unrelated to ubus itself. I could imagine
similar mechanism might be interesting also for d-bus interface
eventually.</p>
<p>Why does it log ACK options? Can they be different for known
host? I think logging network tag would help, but otherwise sent
options should be constant according to configuration. Should it
store those flags again?<br>
</p>
<p>First problem is it always allocates extra info for the lease. I
think it should try it only when --enable-ubus is used in
configuration. I would move separate net member into
ubus_extra_info structure. And move its defintion outside lease
structure, use in struct lease {} just:<br>
</p>
<p>struct ubus_extra_info *ubus_extra_info;</p>
<p>I would use functions to free and reset ubus_extra_info inside
ubus.c. It might not need to know structure definition of
ubus_extra_info. Avoid complex ubus_delete_options() and free in
lease.c, move it to function called
ubus_free_extra_info(lease->ubus_extra_info);</p>
<p>Move implementation details to ubus, since those details are only
used there. I don't like extending log_packet by lease, just
because only 3 types are logged with details.</p>
<p>Why is used (string ? string : NULL) ?? use just string. It is
either non-NULL or NULL anyway. I don't like assigning xid in
packet_log function. It should only log, not change the records
also. Is DHCPREQUEST handled by if's, but not supplied with lease
intentional? I would expect options from request to be the most
important to log.</p>
<p>I would use it like this, since it should not log every message:<br>
</p>
<p>log_packet("DHCPREQUEST", ...);<br>
#ifdef HAVE_UBUS<br>
ubus_event_bcast("dhcp.request", daemon->namebuff,
inet_ntoa(&mess->yiaddr), NULL, iface_name, lease);<br>
#endif</p>
<p>Just might thoughts after quick patch review.</p>
<p>Cheers,<br>
Petr<br>
</p>
<div class="moz-cite-prefix">On 6/11/21 11:59 AM, Eduardo AGUILAR
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:PR0P264MB2171F0850BBDF720E49CC347B0349@PR0P264MB2171.FRAP264.PROD.OUTLOOK.COM">
<pre class="moz-quote-pre" wrap="">Hi everyone,
I tried emailing Simon directly but he seems absent.
I send a patch we are using at Soft at Home to retrieve more information about leases using ubus. We would like to have this upstreamed if possible.
We are maintaining these changes at <a class="moz-txt-link-freetext" href="https://gitlab.com/soft.at.home/forks/dnsmasq/-/tree/v2.85_ubus_patch">https://gitlab.com/soft.at.home/forks/dnsmasq/-/tree/v2.85_ubus_patch</a>
Please let me know about any feedback you may have.
Best Regards,
Eduardo.</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
Dnsmasq-discuss mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Dnsmasq-discuss@lists.thekelleys.org.uk">Dnsmasq-discuss@lists.thekelleys.org.uk</a>
<a class="moz-txt-link-freetext" href="https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss">https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss</a>
</pre>
</blockquote>
</body>
</html>