On Wed, May 30, 2012 at 09:35:24PM -0700, Ethan Jackson wrote:
> > It's a pretty low value, but it's enough to allow us to estimate the top
> > sources of packets in any given channel, which is the goal.
>
> > 256 would make sense if we were goal were to log for later data-mining
> > type analysis. 8 seems more reasonable for the kind of "rule of thumb"
> > manual analysis that I actually do in practice when someone reports
> > problems. "What's the top source of packets?" is the question I ask,
> > followed by "Any other sources with a comparable quantity?" and the
> > logging here should answer both questions nicely.
> >
> >> > +/* Logs information about a packet that was recently lost in 'ch' (in
> >> > + * 'dpif_'). */
> >> > +static void
> >> > +report_loss(struct dpif *dpif_, struct dpif_channel *ch)
> >>
> >> I think it would be quite useful to expose this information in an
> >> appctl command. May also be useful to automatically log this every
> >> once in a while even if we aren't experiencing problems. Neither of
> >> these suggestions should block merging of this patch, just a thought.
> >
> > I agree with the first, not sure about the latter.
> >
> >> > + VLOG_ERR("%s: lost packet with hash %d%s",
> >> > + dpif_name(dpif_), ch - dpif->channels, ds_cstr(&s));
> >>
> >> I don't fully understand this log message. Why is ch - dpif->channels
> >> called a hash? I don't see how that information is useful either. I
> >> would think just the dpif_name and the dynamic-string would be
> >> sufficient.
> >
> > ch - dpif->channels is a hash of the port number because the channels
> > are assigned as a function of the port number. Would s/with hash/in
> > channel/ make it clearer?
>
> Sounds reasonable.
I pushed this to master for now. I'll cherry-pick to 1.7 in a few days
unless I hear of trouble reports. (I don't expect them but I'm on
vacation so I won't have a chance to respond immediately if they
happen.)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
