Hi Murphy, this is an addition to the previous patch (I already pushed
previous patch to our group's pox repo). You may feel to squash them though.
Peter
>From 88cc0611dda88e46eea572032a95199abd07e147 Mon Sep 17 00:00:00 2001
From: Peter Peresini <peter.peres...@gmail.com>
Date: Thu, 24 Jul 2014 08:22:52 +0200
Subject: [PATCH] More idiomatic logging in ipv4.py - use arguments instead
of
directly formatting the string
---
pox/lib/packet/ipv4.py | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/pox/lib/packet/ipv4.py b/pox/lib/packet/ipv4.py
index 6c47a2e..09428d8 100644
--- a/pox/lib/packet/ipv4.py
+++ b/pox/lib/packet/ipv4.py
@@ -106,7 +106,8 @@ class ipv4(packet_base):
self.raw = raw
dlen = len(raw)
if dlen < ipv4.MIN_LEN:
- self.msg('warning IP packet data too short to parse header:
data len %u' % (dlen,))
+ self.msg('warning IP packet data too short to parse header: '
+ 'data len %u', dlen)
return
(vhl, self.tos, self.iplen, self.id, self.frag, self.ttl,
@@ -123,18 +124,18 @@ class ipv4(packet_base):
self.srcip = IPAddr(self.srcip)
if self.v != ipv4.IPv4:
- self.msg('(ip parse) warning: IP version %u not IPv4' % self.v)
+ self.msg('(ip parse) warning: IP version %u not IPv4', self.v)
return
- if self.hl < 5:
- self.msg('(ip parse) warning: IP header length shorter than
MIN_LEN (IHL=%u => header len=%u)' \
- % (self.hl, 4 * self.hl))
+ if (self.hl * 4) < ipv4.MIN_LEN:
+ self.msg('(ip parse) warning: IP header length shorter than
MIN_LEN '
+ '(IHL=%u => header len=%u)', self.hl, 4 * self.hl)
return
if self.iplen < ipv4.MIN_LEN:
- self.msg('(ip parse) warning: Invalid IP len %u' % self.iplen)
+ self.msg('(ip parse) warning: Invalid IP len %u', self.iplen)
return
if (self.hl * 4) > self.iplen:
- self.msg('(ip parse) warning: IP header longer than IP length
including payload (%u vs %u)' \
- % (self.hl, self.iplen))
+ self.msg('(ip parse) warning: IP header longer than IP length '
+ 'including payload (%u vs %u)', self.hl, self.iplen)
return
if (self.hl * 4) > dlen:
self.msg('(ip parse) warning: IP header is truncated')
@@ -156,7 +157,8 @@ class ipv4(packet_base):
elif self.protocol == ipv4.IGMP_PROTOCOL:
self.next = igmp(raw=raw[self.hl*4:length], prev=self)
elif dlen < self.iplen:
- self.msg('(ip parse) warning IP packet data shorter than IP
len: %u < %u' % (dlen, self.iplen))
+ self.msg('(ip parse) warning IP packet data shorter than IP
len '
+ '(%u < %u)', dlen, self.iplen)
else:
self.next = raw[self.hl*4:length]
--
1.8.3.2
On Thu, Jul 24, 2014 at 5:33 AM, Peter Peresini <peter.peres...@epfl.ch>
wrote:
> True, as a part of logging infrastructure the last version is preferred,
> will do that...
>
> Peter
> (sent from android device)
> On Jul 23, 2014 8:34 PM, "Murphy McCauley" <murphy.mccau...@gmail.com>
> wrote:
>
>> This looks good to me, though I wonder if you'd be willing to do a little
>> more while you're at it. Good POX code should always use a literal tuple
>> when using the string formatting operator. This may not actually be on the
>> wiki, but it should be. The packet library predates POX and its
>> conventions, though, so there's plenty of code in there which breaks the
>> rule.
>>
>> So basically, things like...
>> self.msg('(ip parse) warning: IP version %u not IPv4' % self.v)
>>
>> should be...
>> self.msg('(ip parse) warning: IP version %u not IPv4' % (self.v,))
>>
>> Or, better yet, I think the following would work:
>> self.msg('(ip parse) warning: IP version %u not IPv4', self.v)
>>
>> And for that matter, I don't think there's any reason not to do:
>> self.msg('(ip parse) warning: IP version %s not IPv4', self.v)
>>
>> .. which is less likely to have an attempt at a helpful log message
>> result in an exception instead.
>>
>>
>> If you don't have the time and inclination, let me know. Just seemed to
>> me like as long as you're fixing things... might as well fix them the rest
>> of the way.
>>
>> (And I'll respond on the subject of addresses.py eventually; just haven't
>> quite had the time to yet.)
>>
>> -- Murphy
>>
>> On Jul 22, 2014, at 1:05 AM, Peter Peresini <peter.peres...@epfl.ch>
>> wrote:
>>
>> Hi Murphy,
>> here is a small patch that improves warning messages generated during
>> IPv4 parsing.
>>
>> Peter
>>
>>
>> From 7b5f395bd49696290d647b08db271a4ca0e06e43 Mon Sep 17 00:00:00 2001
>> From: Peter Peresini <peter.peres...@gmail.com>
>> Date: Tue, 22 Jul 2014 10:02:18 +0200
>> Subject: [PATCH] Improved warning messages during IPv4 parsing
>>
>> ---
>> pox/lib/packet/ipv4.py | 19 +++++++++++--------
>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/pox/lib/packet/ipv4.py b/pox/lib/packet/ipv4.py
>> index 3eb4ae6..6c47a2e 100644
>> --- a/pox/lib/packet/ipv4.py
>> +++ b/pox/lib/packet/ipv4.py
>> @@ -123,19 +123,22 @@ class ipv4(packet_base):
>> self.srcip = IPAddr(self.srcip)
>>
>> if self.v != ipv4.IPv4:
>> - self.msg('(ip parse) warning IP version %u not IPv4' %
>> self.v)
>> + self.msg('(ip parse) warning: IP version %u not IPv4' %
>> self.v)
>> return
>> - elif self.hl < 5:
>> - self.msg('(ip parse) warning IP header %u longer than len
>> %u' \
>> - % (self.hl, self.iplen))
>> + if self.hl < 5:
>> + self.msg('(ip parse) warning: IP header length shorter than
>> MIN_LEN (IHL=%u => header len=%u)' \
>> + % (self.hl, 4 * self.hl))
>> return
>> - elif self.iplen < ipv4.MIN_LEN:
>> - self.msg('(ip parse) warning invalid IP len %u' % self.iplen)
>> + if self.iplen < ipv4.MIN_LEN:
>> + self.msg('(ip parse) warning: Invalid IP len %u' %
>> self.iplen)
>> return
>> - elif (self.hl * 4) >= self.iplen or (self.hl * 4) > dlen:
>> - self.msg('(ip parse) warning IP header %u longer than len
>> %u' \
>> + if (self.hl * 4) > self.iplen:
>> + self.msg('(ip parse) warning: IP header longer than IP
>> length including payload (%u vs %u)' \
>> % (self.hl, self.iplen))
>> return
>> + if (self.hl * 4) > dlen:
>> + self.msg('(ip parse) warning: IP header is truncated')
>> + return
>>
>> # At this point, we are reasonably certain that we have an IP
>> # packet
>> --
>> 1.8.3.2
>>
>>
>>
