Hello, On Tue, 14 Jun 2016, Quentin Armitage wrote: > This series of patches arise from discovering that: > ipvsadm --start-daemon backup --mcast-group IPv6_address ... > would always fail. > > The first patch resolves the problem. The second and third patches are > optimizations that were noticed while investigating the original problem. > The fourth patch adds a lock which appears to have been omitted, and the > final patch adds the recently added sync daemon multicast parameters to > the log messages that are written when the sync daemons start. > > v2 fixes a compile error in a debug message identified by kbuild test robot. > Now compiles with CONFIG_IP_VS_DEBUG enabled. Patch 2/5 is modified to correct > the problem, and patch 3/5 is modifed to apply with the modified patch 2/5. > > Quentin Armitage (5): > ipvs: Enable setting IPv6 multicast address for ipvs sync daemon > backup > ipvs: Stop calling __dev_get_by_name() repeatedly when starting sync > daemon > ipvs: Don't check result < 0 after setting result = 0 > ipvs: Lock socket before setting SK_CAN_REUSE > ipvs: log additional sync daemon parameters > > net/netfilter/ipvs/ip_vs_sync.c | 104 +++++++++++++++++++------------------- > 1 files changed, 52 insertions(+), 52 deletions(-) > > -- > 1.7.7.6 Thanks for catching this bug. Following are my comments for the patches: Patch 1: I missed the fact that link-local addresses (ffx2) require binding to ifindex due to __ipv6_addr_needs_scope_id check, I tested only with a ff05 address. BTW, ff01 is a node-local address (loopback), you should not use it for IPVS. Instead of directly writing into sin6_scope_id we can use 'sock->sk->sk_bound_dev_if = ifindex;' before bind(), it will work for v4 and v6. Let me know if such solution works. You have to send this patch as a bugfix, it should apply to the net tree and later will go to stable trees (4.3+), i.e. 4.4, 4.5, 4.6 and 4.7, I don't see stable 4.3 in https://www.kernel.org/. You should mention in commit message that this patch is a fix to specific commit (check Documentation/SubmittingPatches): Fixes: d33288172e72 ("ipvs: add more mcast parameters for the sync daemon") The other patches will go to the net-next tree in separate patchset but I see little fuzz if patch 2 is applied without patch 1, so may be this patchset should wait the first patch to appear in net-next kernel. Patch 2: looks OK Patch 3: looks OK It was done this way to not exceed the 80-char limit. May be you can reduce the message for the same reason. Patch 4: looks OK Before bind() such operations should be safe without locks. Patch 5: No need of <> for the commit IDs. The indentation of existing pr_info in both cases should not be changed. Patches 1, 2, 3 have coding style warnings from checkpatch that can be fixed, you can check them in this way: scripts/checkpatch.pl --strict /tmp/file.patch Regards -- Julian Anastasov <ja@xxxxxx> -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
