[GitHub] [trafficserver] bneradt edited a comment on issue #6405: LogFilterIP appears to be broken

[GitBox <gi...@apache.org>]

bneradt edited a comment on issue #6405:
URL: https://github.com/apache/trafficserver/issues/6405#issuecomment-898539171


   Stepping through with a debugger, it looks like the marshaled `value` from the field has the expected value:
   
   ```
   (gdb) f
   #0  LogFilterIP::is_match (this=0x30c7ec0, lad=0x7ff4a6b6af40) at LogFilter.cc:840
   840         zret = m_map.contains(reinterpret_cast<IpAddr &>(value));
   (gdb) l
   835       if (m_field && lad) {
   836         LogFieldIpStorage value;
   837         m_field->marshal(lad, reinterpret_cast<char *>(&value));
   838         // This is bad, we abuse the fact that the initial layout of LogFieldIpStorage and IpAddr
   839         // are identical. We should look at converting the log stuff to use IpAddr directly.
   840         zret = m_map.contains(reinterpret_cast<IpAddr &>(value));
   841       }
   842
   843       return zret;
   844     }
   (gdb) p value
   $11 = {
     _ip = {
       _family = 2
     }, 
     _ip4 = {
       <LogFieldIp> = {
         _family = 2
       }, 
       members of LogFieldIp4: 
       _addr = 16777343  // <--- This corresponds to 127.0.0.1
     }, 
     _ip6 = {
       <LogFieldIp> = {
         _family = 2
       }, 
       members of LogFieldIp6: 
       _addr = {
         __in6_u = {
           __u6_addr8 = "\177\000\000\001@\000\000\000\000\000\000\000@\000\000", 
           __u6_addr16 = {127, 256, 64, 0, 0, 0, 64, 0}, 
           __u6_addr32 = {16777343, 64, 0, 64}
         }
       }
     }
   }
   ```
   
   But the m_map of the local LogFilterIP is empty:
   
   ```
   (gdb) p m_map
   $12 = {
     static RAW_IP4_MIN_ADDR = 0, 
     static IP4_MIN_ADDR = {
       _family = 2, 
       _addr = {
         _ip4 = 0, 
         _ip6 = {
           __in6_u = {
             __u6_addr8 = '\000' <repeats 15 times>, 
             __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, 
             __u6_addr32 = {0, 0, 0, 0}
           }
         }, 
         _byte = '\000' <repeats 15 times>, 
         _u32 = {0, 0, 0, 0}, 
         _u64 = {0, 0}
       }, 
       static INVALID = {
         _family = 0, 
         _addr = {
           _ip4 = 0, 
           _ip6 = {
             __in6_u = {
               __u6_addr8 = '\000' <repeats 15 times>, 
               __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, 
               __u6_addr32 = {0, 0, 0, 0}
             }
           }, 
           _byte = '\000' <repeats 15 times>, 
           _u32 = {0, 0, 0, 0}, 
           _u64 = {0, 0}
         }, 
         static INVALID = <same as static member of an already seen type>
       }
     }, 
     static RAW_IP4_MAX_ADDR = 4294967295, 
     static IP4_MAX_ADDR = <same as static member of an already seen type>, 
     static RAW_IP6_MIN_ADDR = <same as static member of an already seen type>, 
     static IP6_MIN_ADDR = <same as static member of an already seen type>, 
     static RAW_IP6_MAX_ADDR = <same as static member of an already seen type>, 
     static IP6_MAX_ADDR = <same as static member of an already seen type>, 
     _m4 = 0x30c7f60, 
     _m6 = 0x0
   }
   ```
   
   ----
   
   ### Update
   
   Correction: I missed that LogFilterIP was indeed updated in `m_map` via `_m4` which is non-null (`0x30c7f60`, in the above case).
   
   I wonder now whether the issue has to do with this comment:
   
   https://github.com/apache/trafficserver/blob/fb9b9ca39a6f39ad95964f2899b8398c26453532/proxy/logging/LogFilter.cc#L838-#L840
   
   The `LogFieldIpStorage` has not changed since 2011, but IpAddr's union has changed since that comment has gone in (2014) and it seems, by the behavior we're seeing, that it no longer matches against LogFieldIpStorage.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org