On Wed, 2008-11-12 at 21:40 -0500, Peter Memishian wrote: > > > I've found the problem and have come up with a fix which I've tested. > > > The problem was with the protocol version matching in the ipnet header. > > > The protocol field isn't a 2-byte field at offset 0, but a 1 byte field > > > at offset 1. > > > > > > http://zhadum.east/ws/seb/seb-onfix/webrev/ > > > > > > I'd like to get both of these fixes RTI'ed today if possible. Meem, can > > > you look this over, I believe that Phil is offline at this point? Phil > > > has already reviewed the fix to 6770479 in snoop_pf.c, so what's left to > > > be reviewed is the fix to 6770744 in snoop_filter.c and snoop_ether.c. > > I don't fully understand the change to match_types[]; why is it more > correct to ignore the version field? Phrased differently: suppose we want > to rev the dl_ipnetinfo_t version; how would snoop know how to distinguish > between the two versions?
Mathing the version field there was rather arbitrary, as there is also code that matches the zones fields without checking the version field. Should there be a version check in every place where a field is matched? The overall size of the header is also related to the version, and the filters are also dependent on that. So the existing version check was arbitrary and inadequate. My take is that fixing the integrity of the output of snoop filters in the event that someone changes the ipnet version and header format without modifying snoop is outside the scope of this bug fix. Perhaps a separate bug should be filed for that. -Seb
