| Message ID | 20240405005510.19778-1-newtwen+github@gmail.com |
|---|---|
| Headers | show |
| Series | odhcpd patchset | expand |
On Fri, Apr 05, 2024 at 02:53:03AM +0200, Paul Donald wrote: > From: Paul Donald <newtwen@gmail.com> > > refactor and fix limit prefix preferred_lt to valid_lt in accordance with RFC4861 All changes look good and I generally agree. Thank you! Please avoid duplicate commit titles ("various: refactor") as well as empty commit messages as that makes the project git history harder to read and understand. Also, in case of automated refactoring it would be great if you can include the method (e.g. sed script) used for carrying them out in the commit message. I know they are trivial and the those scripts can easily be inferred by reviewers, however, having a sed-script and apply that locally, then compare it with your suggested patch is easier than reviewing the patch itself (esp. when it comes to accidental ommissions). > > Tested on 23.05.0 I assume the patchset is intended to be applied on the current git HEAD of odhcpd.git, right? Also that is something worth mentioning in the cover letter. For the whole series: Reviewed-by: Daniel Golle <daniel@makrotopia.org> > > Paul Donald (9): > various: refactor > various: refactor > various: Comment fixes > router: inherit user-assigned preferred_lifetime > router: Limit prefix preferred_lt to valid_lt in accordance with > RFC4861 > router: Apply updated values from RFC8319 (updates RFC4861) > config: ra_management is deprecated comment > router: Type comments > ndp: Comments > > src/config.c | 1 + > src/dhcpv4.c | 2 +- > src/dhcpv6-ia.c | 140 ++++++++++++++++++++++++------------------------ > src/dhcpv6.c | 6 +-- > src/dhcpv6.h | 8 +-- > src/ndp.c | 4 +- > src/netlink.c | 56 +++++++++---------- > src/odhcpd.c | 8 +-- > src/odhcpd.h | 4 +- > src/router.c | 72 ++++++++++++++----------- > src/router.h | 21 +++++++- > 11 files changed, 176 insertions(+), 146 deletions(-) > > -- > 2.44.0 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 2024-04-05 03:34, Daniel Golle wrote: > On Fri, Apr 05, 2024 at 02:53:03AM +0200, Paul Donald wrote: >> From: Paul Donald <newtwen@gmail.com> >> >> refactor and fix limit prefix preferred_lt to valid_lt in accordance with RFC4861 > > All changes look good and I generally agree. Thank you! > Good. Thank you. :) At this point (after amendments) is a new patch-set appropriate? BTW who should one generally CC: for odhcpd? > Please avoid duplicate commit titles ("various: refactor") as well as > empty commit messages as that makes the project git history harder to > read and understand. Understood. I was at the risk of repeating myself, since the subject and 'body' line of the short commit message would basically be the same. The openwrt main repo has some automated checks on github that don't allow no body. > Also, in case of automated refactoring it would be great if you can > include the method (e.g. sed script) used for carrying them out in the > commit message. I know they are trivial and the those scripts can > easily be inferred by reviewers, however, having a sed-script and > apply that locally, then compare it with your suggested patch is > easier than reviewing the patch itself (esp. when it comes to > accidental ommissions). I worked through it manually - it varied across files. Although I can see your value in automation. Are there any you recommend? >> >> Tested on 23.05.0 > > I assume the patchset is intended to be applied on the current git > HEAD of odhcpd.git, right? Also that is something worth mentioning in > the cover letter. Good point. Yes. odhcpd has not changed since I wrote the patchset. Before: == ICMPv6 Option (Prefix information : fd51:1c2a:8909::/64) Type: Prefix information (3) Length: 4 (32 bytes) Prefix Length: 64 Flag: 0xc0, On-link flag(L), Autonomous address-configuration flag(A) Valid Lifetime: Infinity (4294967295) Preferred Lifetime: Infinity (4294967295) Reserved Prefix: fd51:1c2a:8909:: ==After:== ICMPv6 Option (Prefix information : fd51:1c2a:8909::/64) Type: Prefix information (3) Length: 4 (32 bytes) Prefix Length: 64 Flag: 0xc0, On-link flag(L), Autonomous address-configuration flag(A) Valid Lifetime: Infinity (4294967295) Preferred Lifetime: 420 Reserved Prefix: fd51:1c2a:8909:: == > For the whole series: > Reviewed-by: Daniel Golle <daniel@makrotopia.org>
From: Paul Donald <newtwen@gmail.com> refactor and fix limit prefix preferred_lt to valid_lt in accordance with RFC4861 Tested on 23.05.0 Paul Donald (9): various: refactor various: refactor various: Comment fixes router: inherit user-assigned preferred_lifetime router: Limit prefix preferred_lt to valid_lt in accordance with RFC4861 router: Apply updated values from RFC8319 (updates RFC4861) config: ra_management is deprecated comment router: Type comments ndp: Comments src/config.c | 1 + src/dhcpv4.c | 2 +- src/dhcpv6-ia.c | 140 ++++++++++++++++++++++++------------------------ src/dhcpv6.c | 6 +-- src/dhcpv6.h | 8 +-- src/ndp.c | 4 +- src/netlink.c | 56 +++++++++---------- src/odhcpd.c | 8 +-- src/odhcpd.h | 4 +- src/router.c | 72 ++++++++++++++----------- src/router.h | 21 +++++++- 11 files changed, 176 insertions(+), 146 deletions(-)