[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openvswitch-dev
Subject:    [ovs-dev] [loss-report 4/4] dpif-linux: Log details when a packet is lost.
From:       ethan () nicira ! com (Ethan Jackson)
Date:       2012-05-31 4:35:24
Message-ID: CAEruU62tOjKfA0WHCa2A3a0bFu_AcPDgz=urw79YKLW6hUqEvA () mail ! gmail ! com
[Download RAW message or body]

> 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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic