Hannes Eder wrote:
> On Wed, Sep 2, 2009 at 16:54, Patrick McHardy<kaber@xxxxxxxxx> wrote:
>> Hannes Eder wrote:
>>> This implements the kernel-space side of the netfilter matcher
>>> xt_ipvs.
>> Looks mostly fine to me, just one question:
>>
>>> +bool ipvs_mt(const struct sk_buff *skb, const struct xt_match_param *par)
>>> +{
>>> + const struct xt_ipvs *data = par->matchinfo;
>>> + const u_int8_t family = par->family;
>>> + struct ip_vs_iphdr iph;
>>> + struct ip_vs_protocol *pp;
>>> + struct ip_vs_conn *cp;
>>> + int af;
>>> + bool match = true;
>>> +
>>> + if (data->bitmask == XT_IPVS_IPVS_PROPERTY) {
>>> + match = skb->ipvs_property ^
>>> + !!(data->invert & XT_IPVS_IPVS_PROPERTY);
>>> + goto out;
>>> + }
>>> +
>>> + /* other flags than XT_IPVS_IPVS_PROPERTY are set */
>>> + if (!skb->ipvs_property) {
>>> + match = false;
>>> + goto out;
>>> + }
>>> +
>>> + switch (skb->protocol) {
>>> + case htons(ETH_P_IP):
>>> + af = AF_INET;
>>> + break;
>>> +#ifdef CONFIG_IP_VS_IPV6
>>> + case htons(ETH_P_IPV6):
>>> + af = AF_INET6;
>>> + break;
>>> +#endif
>>> + default:
>>> + match = false;
>>> + goto out;
>>> + }
>> In the NF_INET_LOCAL_OUT hook skb->protocol is invalid. So if you
>> don't need this, it would make sense to restrict the match to the
>> other hooks.
>>
>> Even easier would be to use par->family, which contains the address
>> family and doesn't need any translation.
>
> Nice, I'll use par->family.
>
> So in theory I do not even need a check like the following in the beginning?
>
> if (family != NFPROTO_IPV4
> #ifdef CONFIG_IP_VS_IPV6
> && family != NFPROTO_IPV6
> #endif
> ) {
> match = false;
> goto out;
> }
With the AF_UNSPEC registration of your match, it might be used
with different families. But you could add two seperate IPV4/IPV6
registrations or catch an invalid family in ->checkentry() and
remove the runtime check.
--
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
