> 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.

Ethan
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to