> > > > 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.
It may have been inadequate, but the spirit was correct: we're trying to determine whether or not the header has the expected format. We can't do that without checking the dl_ipnetinfo_t version somewhere. > 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. Sigh. OK. -- meem
