[ovs-dev] [PATCH] lib/conntrack: remove unnecessary addr check for ICMP.

Darrell Ball dball at vmware.com
Thu Dec 7 02:13:48 UTC 2017


Hi Wang

To speed up the process, I sent an alternative patch here:

https://patchwork.ozlabs.org/patch/845407/

I agree the address sanity check is not correct but I think it should be partially retained
rather than removed. I also think a test was needed.

Pls let me know if it makes sense.
Also. if you prefer, I can make you the author.

Thanks Darrell




On 12/6/17, 11:22 AM, "Darrell Ball" <dball at vmware.com> wrote:

    Thanks for looking at this.
    
    In the commit message, can you delineate.
    
    1/ The forward direction packet in terms of src ip, dest ip, L4 attributes
    2/ The reverse direction error packet in terms of src ip, dest ip, icmp error payload
    
    Darrell
    
    On 12/4/17, 10:22 PM, "ovs-dev-bounces at openvswitch.org on behalf of wang zhike" <ovs-dev-bounces at openvswitch.org on behalf of wangzhike at jd.com> wrote:
    
        From: wangzhike <wangzhike at jd.com>
        
        ICMP response (Unreachable/fragmentationRequired/...) may be created
        at devices in the middle, and such packets are tagged as invalid in
        user space conntrack. In fact it does not make sense to validate the
        src and dest address.
        
        Signed-off-by: wang zhike <wangzhike at jd.com>
        ---
         lib/conntrack.c | 13 -------------
         1 file changed, 13 deletions(-)
        
        diff --git a/lib/conntrack.c b/lib/conntrack.c
        index f5a3aa9..c44ad0f 100644
        --- a/lib/conntrack.c
        +++ b/lib/conntrack.c
        @@ -1702,11 +1702,6 @@ extract_l4_icmp(struct conn_key *key, const void *data, size_t size,
                     return false;
                 }
         
        -        if (inner_key.src.addr.ipv4_aligned != key->dst.addr.ipv4_aligned
        -            || inner_key.dst.addr.ipv4_aligned != key->src.addr.ipv4_aligned) {
        -            return false;
        -        }
        -
                 key->src = inner_key.src;
                 key->dst = inner_key.dst;
                 key->nw_proto = inner_key.nw_proto;
        @@ -1789,14 +1784,6 @@ extract_l4_icmp6(struct conn_key *key, const void *data, size_t size,
                     return false;
                 }
         
        -        /* pf doesn't do this, but it seems a good idea */
        -        if (!ipv6_addr_equals(&inner_key.src.addr.ipv6_aligned,
        -                              &key->dst.addr.ipv6_aligned)
        -            || !ipv6_addr_equals(&inner_key.dst.addr.ipv6_aligned,
        -                                 &key->src.addr.ipv6_aligned)) {
        -            return false;
        -        }
        -
                 key->src = inner_key.src;
                 key->dst = inner_key.dst;
                 key->nw_proto = inner_key.nw_proto;
        -- 
        1.8.3.1
        
        _______________________________________________
        dev mailing list
        dev at openvswitch.org
        https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=IqOQ4UcLnOpQUkvP7FHVH6IWeAK_4DDBRBqX2w3wl94&s=Xmvgnl8plChpjcr77sLIOxE0krKVuNVqvS3_eOoNeMg&e=
        
    
    



More information about the dev mailing list