Hello,
Comments about v2:
Patch 1:
This line should not be removed:
- writable = ciph.len;
it should be 'offset = ciph.len;'
Patch 2: OK
Patch 3: OK
Patch 4: OK
Patch 5:
We provide correct ports to ip_vs_sched_persist() but I guess
it needs to be updated for the address usage just like
ip_vs_schedule.
Patch 6: OK
Patch 7: OK
Patch 8:
CHECK: Comparison to NULL could be written "!ports"
#45: FILE: net/netfilter/ipvs/ip_vs_sh.c:296:
+ if (unlikely(ports == NULL))
In the debug message in ip_vs_sh_schedule we can use
hash_addr instead of &iph->saddr
Patch 9:
- ip_vs_in_icmp and ip_vs_in_icmp_v6:
- Now this check is not needed anymore:
if (unlikely(!cp))
return NF_ACCEPT;
Patch 10: OK
Patch 11: OK
Patch 12: OK
Patch 13: OK
Looking at tcp_conn_schedule I see that ip_vs_leave() can be
called. We should add checks in ip_vs_leave(), for example:
'!ip_vs_iph_icmp(iph)' in:
if (ipvs->sysctl_cache_bypass && svc->fwmark && unicast) {
i.e. we should not use the bypass feature for ICMP, there are
calls to ip_vs_set_state() and cp->packet_xmit() that are
not allowed for ICMP packets without adding more checks.
Then the 'if ((svc->port == FTPPORT) && (pptr[1] != FTPPORT))'
block should be extended to use pptr[0] for ICMP packet.
Then such check should follow to avoid sending ICMP to ICMP:
if (ip_vs_iph_icmp(iph))
return NF_DROP;
Also, I guess the connections created by ICMP packets use
the cp->state = 0 and cp->timeout = 3*HZ defaults from ip_vs_conn_new,
this should be fine, right?
Regards
--
Julian Anastasov <ja@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|