Committed by
Ray Milkey
[ONOS-4931] fix doc of FlowEntry.life(), add life(TimeUnit)
Fix the doc: life() returns the time in seconds, not milliseconds. Add new method life(TimeUnit) that allows specifying the timeunit to receive the life value as as seconds might not be enough for all applications and OpenFlow can provide this value to nanoseconds resolution (in its spec). Change-Id: Ia6a7573797249e0edc04e03c7204a550a2823742
Showing
10 changed files
with
92 additions
and
25 deletions
... | @@ -16,16 +16,22 @@ | ... | @@ -16,16 +16,22 @@ |
16 | package org.onosproject.net.flow; | 16 | package org.onosproject.net.flow; |
17 | 17 | ||
18 | import static com.google.common.base.MoreObjects.toStringHelper; | 18 | import static com.google.common.base.MoreObjects.toStringHelper; |
19 | +import static java.util.concurrent.TimeUnit.NANOSECONDS; | ||
20 | +import static java.util.concurrent.TimeUnit.SECONDS; | ||
19 | import static org.slf4j.LoggerFactory.getLogger; | 21 | import static org.slf4j.LoggerFactory.getLogger; |
20 | 22 | ||
21 | import org.slf4j.Logger; | 23 | import org.slf4j.Logger; |
22 | 24 | ||
25 | +import java.util.concurrent.TimeUnit; | ||
26 | + | ||
23 | public class DefaultFlowEntry extends DefaultFlowRule | 27 | public class DefaultFlowEntry extends DefaultFlowRule |
24 | implements StoredFlowEntry { | 28 | implements StoredFlowEntry { |
25 | 29 | ||
26 | private static final Logger log = getLogger(DefaultFlowEntry.class); | 30 | private static final Logger log = getLogger(DefaultFlowEntry.class); |
27 | 31 | ||
32 | + /* Stored in nanoseconds (allows for 292 years) */ | ||
28 | private long life; | 33 | private long life; |
34 | + | ||
29 | private long packets; | 35 | private long packets; |
30 | private long bytes; | 36 | private long bytes; |
31 | private FlowEntryState state; | 37 | private FlowEntryState state; |
... | @@ -37,10 +43,10 @@ public class DefaultFlowEntry extends DefaultFlowRule | ... | @@ -37,10 +43,10 @@ public class DefaultFlowEntry extends DefaultFlowRule |
37 | private final int errCode; | 43 | private final int errCode; |
38 | 44 | ||
39 | public DefaultFlowEntry(FlowRule rule, FlowEntryState state, | 45 | public DefaultFlowEntry(FlowRule rule, FlowEntryState state, |
40 | - long life, long packets, long bytes) { | 46 | + long life, TimeUnit lifeTimeUnit, long packets, long bytes) { |
41 | super(rule); | 47 | super(rule); |
42 | this.state = state; | 48 | this.state = state; |
43 | - this.life = life; | 49 | + this.life = lifeTimeUnit.toNanos(life); |
44 | this.packets = packets; | 50 | this.packets = packets; |
45 | this.bytes = bytes; | 51 | this.bytes = bytes; |
46 | this.errCode = -1; | 52 | this.errCode = -1; |
... | @@ -48,15 +54,13 @@ public class DefaultFlowEntry extends DefaultFlowRule | ... | @@ -48,15 +54,13 @@ public class DefaultFlowEntry extends DefaultFlowRule |
48 | this.lastSeen = System.currentTimeMillis(); | 54 | this.lastSeen = System.currentTimeMillis(); |
49 | } | 55 | } |
50 | 56 | ||
57 | + public DefaultFlowEntry(FlowRule rule, FlowEntryState state, | ||
58 | + long lifeSecs, long packets, long bytes) { | ||
59 | + this(rule, state, lifeSecs, SECONDS, packets, bytes); | ||
60 | + } | ||
61 | + | ||
51 | public DefaultFlowEntry(FlowRule rule) { | 62 | public DefaultFlowEntry(FlowRule rule) { |
52 | - super(rule); | 63 | + this(rule, FlowEntryState.PENDING_ADD, 0, 0, 0); |
53 | - this.state = FlowEntryState.PENDING_ADD; | ||
54 | - this.life = 0; | ||
55 | - this.packets = 0; | ||
56 | - this.bytes = 0; | ||
57 | - this.errCode = -1; | ||
58 | - this.errType = -1; | ||
59 | - this.lastSeen = System.currentTimeMillis(); | ||
60 | } | 64 | } |
61 | 65 | ||
62 | public DefaultFlowEntry(FlowRule rule, int errType, int errCode) { | 66 | public DefaultFlowEntry(FlowRule rule, int errType, int errCode) { |
... | @@ -69,7 +73,12 @@ public class DefaultFlowEntry extends DefaultFlowRule | ... | @@ -69,7 +73,12 @@ public class DefaultFlowEntry extends DefaultFlowRule |
69 | 73 | ||
70 | @Override | 74 | @Override |
71 | public long life() { | 75 | public long life() { |
72 | - return life; | 76 | + return life(SECONDS); |
77 | + } | ||
78 | + | ||
79 | + @Override | ||
80 | + public long life(TimeUnit timeUnit) { | ||
81 | + return timeUnit.convert(life, NANOSECONDS); | ||
73 | } | 82 | } |
74 | 83 | ||
75 | @Override | 84 | @Override |
... | @@ -104,7 +113,12 @@ public class DefaultFlowEntry extends DefaultFlowRule | ... | @@ -104,7 +113,12 @@ public class DefaultFlowEntry extends DefaultFlowRule |
104 | 113 | ||
105 | @Override | 114 | @Override |
106 | public void setLife(long life) { | 115 | public void setLife(long life) { |
107 | - this.life = life; | 116 | + setLife(life, SECONDS); |
117 | + } | ||
118 | + | ||
119 | + @Override | ||
120 | + public void setLife(long life, TimeUnit timeUnit) { | ||
121 | + this.life = timeUnit.toNanos(life); | ||
108 | } | 122 | } |
109 | 123 | ||
110 | @Override | 124 | @Override | ... | ... |
... | @@ -16,7 +16,10 @@ | ... | @@ -16,7 +16,10 @@ |
16 | 16 | ||
17 | package org.onosproject.net.flow; | 17 | package org.onosproject.net.flow; |
18 | 18 | ||
19 | +import java.util.concurrent.TimeUnit; | ||
20 | + | ||
19 | import static com.google.common.base.MoreObjects.toStringHelper; | 21 | import static com.google.common.base.MoreObjects.toStringHelper; |
22 | +import static java.util.concurrent.TimeUnit.NANOSECONDS; | ||
20 | 23 | ||
21 | /** | 24 | /** |
22 | * Default flow entry class with FlowLiveType value, IMMEDIATE_FLOW, SHORT_FLOW, MID_FLOW, LONG_FLOW. | 25 | * Default flow entry class with FlowLiveType value, IMMEDIATE_FLOW, SHORT_FLOW, MID_FLOW, LONG_FLOW. |
... | @@ -25,6 +28,23 @@ public class DefaultTypedFlowEntry extends DefaultFlowEntry | ... | @@ -25,6 +28,23 @@ public class DefaultTypedFlowEntry extends DefaultFlowEntry |
25 | implements TypedStoredFlowEntry { | 28 | implements TypedStoredFlowEntry { |
26 | private FlowLiveType liveType; | 29 | private FlowLiveType liveType; |
27 | 30 | ||
31 | + | ||
32 | + /** | ||
33 | + * Creates a typed flow entry from flow rule and its statistics, with default flow live type(IMMEDIATE_FLOW). | ||
34 | + * | ||
35 | + * @param rule the flow rule | ||
36 | + * @param state the flow state | ||
37 | + * @param life the flow duration since creation | ||
38 | + * @param lifeTimeUnit the time unit of life | ||
39 | + * @param packets the flow packets count | ||
40 | + * @param bytes the flow bytes count | ||
41 | + */ | ||
42 | + public DefaultTypedFlowEntry(FlowRule rule, FlowEntryState state, | ||
43 | + long life, TimeUnit lifeTimeUnit, long packets, long bytes) { | ||
44 | + super(rule, state, life, lifeTimeUnit, packets, bytes); | ||
45 | + this.liveType = FlowLiveType.IMMEDIATE_FLOW; | ||
46 | + } | ||
47 | + | ||
28 | /** | 48 | /** |
29 | * Creates a typed flow entry from flow rule and its statistics, with default flow live type(IMMEDIATE_FLOW). | 49 | * Creates a typed flow entry from flow rule and its statistics, with default flow live type(IMMEDIATE_FLOW). |
30 | * | 50 | * |
... | @@ -59,7 +79,7 @@ public class DefaultTypedFlowEntry extends DefaultFlowEntry | ... | @@ -59,7 +79,7 @@ public class DefaultTypedFlowEntry extends DefaultFlowEntry |
59 | * | 79 | * |
60 | */ | 80 | */ |
61 | public DefaultTypedFlowEntry(FlowEntry fe) { | 81 | public DefaultTypedFlowEntry(FlowEntry fe) { |
62 | - super(fe, fe.state(), fe.life(), fe.packets(), fe.bytes()); | 82 | + super(fe, fe.state(), fe.life(NANOSECONDS), NANOSECONDS, fe.packets(), fe.bytes()); |
63 | this.liveType = FlowLiveType.IMMEDIATE_FLOW; | 83 | this.liveType = FlowLiveType.IMMEDIATE_FLOW; |
64 | } | 84 | } |
65 | 85 | ||
... | @@ -83,7 +103,7 @@ public class DefaultTypedFlowEntry extends DefaultFlowEntry | ... | @@ -83,7 +103,7 @@ public class DefaultTypedFlowEntry extends DefaultFlowEntry |
83 | * | 103 | * |
84 | */ | 104 | */ |
85 | public DefaultTypedFlowEntry(FlowEntry fe, FlowLiveType liveType) { | 105 | public DefaultTypedFlowEntry(FlowEntry fe, FlowLiveType liveType) { |
86 | - super(fe, fe.state(), fe.life(), fe.packets(), fe.bytes()); | 106 | + super(fe, fe.state(), fe.life(NANOSECONDS), NANOSECONDS, fe.packets(), fe.bytes()); |
87 | this.liveType = liveType; | 107 | this.liveType = liveType; |
88 | } | 108 | } |
89 | 109 | ... | ... |
... | @@ -16,6 +16,8 @@ | ... | @@ -16,6 +16,8 @@ |
16 | package org.onosproject.net.flow; | 16 | package org.onosproject.net.flow; |
17 | 17 | ||
18 | 18 | ||
19 | +import java.util.concurrent.TimeUnit; | ||
20 | + | ||
19 | /** | 21 | /** |
20 | * Represents a generalized match & action pair to be applied to | 22 | * Represents a generalized match & action pair to be applied to |
21 | * an infrastucture device. | 23 | * an infrastucture device. |
... | @@ -60,13 +62,20 @@ public interface FlowEntry extends FlowRule { | ... | @@ -60,13 +62,20 @@ public interface FlowEntry extends FlowRule { |
60 | FlowEntryState state(); | 62 | FlowEntryState state(); |
61 | 63 | ||
62 | /** | 64 | /** |
63 | - * Returns the number of milliseconds this flow rule has been applied. | 65 | + * Returns the number of seconds this flow rule has been applied. |
64 | * | 66 | * |
65 | - * @return number of millis | 67 | + * @return number of seconds |
66 | */ | 68 | */ |
67 | long life(); | 69 | long life(); |
68 | 70 | ||
69 | /** | 71 | /** |
72 | + * Returns the time this flow rule has been applied. | ||
73 | + * | ||
74 | + * @return time in the requested {@link TimeUnit} | ||
75 | + */ | ||
76 | + long life(TimeUnit unit); | ||
77 | + | ||
78 | + /** | ||
70 | * Returns the number of packets this flow rule has matched. | 79 | * Returns the number of packets this flow rule has matched. |
71 | * | 80 | * |
72 | * @return number of packets | 81 | * @return number of packets | ... | ... |
... | @@ -16,6 +16,8 @@ | ... | @@ -16,6 +16,8 @@ |
16 | package org.onosproject.net.flow; | 16 | package org.onosproject.net.flow; |
17 | 17 | ||
18 | 18 | ||
19 | +import java.util.concurrent.TimeUnit; | ||
20 | + | ||
19 | public interface StoredFlowEntry extends FlowEntry { | 21 | public interface StoredFlowEntry extends FlowEntry { |
20 | 22 | ||
21 | /** | 23 | /** |
... | @@ -31,9 +33,16 @@ public interface StoredFlowEntry extends FlowEntry { | ... | @@ -31,9 +33,16 @@ public interface StoredFlowEntry extends FlowEntry { |
31 | 33 | ||
32 | /** | 34 | /** |
33 | * Sets how long this entry has been entered in the system. | 35 | * Sets how long this entry has been entered in the system. |
34 | - * @param life epoch time | 36 | + * @param lifeSecs seconds |
37 | + */ | ||
38 | + void setLife(long lifeSecs); | ||
39 | + | ||
40 | + /** | ||
41 | + * Sets how long this entry has been entered in the system. | ||
42 | + * @param life time | ||
43 | + * @param timeUnit unit of time | ||
35 | */ | 44 | */ |
36 | - void setLife(long life); | 45 | + void setLife(long life, TimeUnit timeUnit); |
37 | 46 | ||
38 | /** | 47 | /** |
39 | * Number of packets seen by this entry. | 48 | * Number of packets seen by this entry. | ... | ... |
... | @@ -75,6 +75,9 @@ public class DefaultFlowEntryTest { | ... | @@ -75,6 +75,9 @@ public class DefaultFlowEntryTest { |
75 | assertThat(defaultFlowEntry1.treatment(), is(TREATMENT)); | 75 | assertThat(defaultFlowEntry1.treatment(), is(TREATMENT)); |
76 | assertThat(defaultFlowEntry1.timeout(), is(1)); | 76 | assertThat(defaultFlowEntry1.timeout(), is(1)); |
77 | assertThat(defaultFlowEntry1.life(), is(1L)); | 77 | assertThat(defaultFlowEntry1.life(), is(1L)); |
78 | + assertThat(defaultFlowEntry1.life(TimeUnit.SECONDS), is(1L)); | ||
79 | + assertThat(defaultFlowEntry1.life(TimeUnit.MILLISECONDS), is(1000L)); | ||
80 | + assertThat(defaultFlowEntry1.life(TimeUnit.MINUTES), is(0L)); | ||
78 | assertThat(defaultFlowEntry1.packets(), is(1L)); | 81 | assertThat(defaultFlowEntry1.packets(), is(1L)); |
79 | assertThat(defaultFlowEntry1.bytes(), is(1L)); | 82 | assertThat(defaultFlowEntry1.bytes(), is(1L)); |
80 | assertThat(defaultFlowEntry1.state(), is(FlowEntry.FlowEntryState.ADDED)); | 83 | assertThat(defaultFlowEntry1.state(), is(FlowEntry.FlowEntryState.ADDED)); |
... | @@ -94,13 +97,14 @@ public class DefaultFlowEntryTest { | ... | @@ -94,13 +97,14 @@ public class DefaultFlowEntryTest { |
94 | entry.setState(FlowEntry.FlowEntryState.PENDING_REMOVE); | 97 | entry.setState(FlowEntry.FlowEntryState.PENDING_REMOVE); |
95 | entry.setPackets(11); | 98 | entry.setPackets(11); |
96 | entry.setBytes(22); | 99 | entry.setBytes(22); |
97 | - entry.setLife(33); | 100 | + entry.setLife(33333, TimeUnit.MILLISECONDS); |
98 | 101 | ||
99 | assertThat(entry.deviceId(), is(did("id1"))); | 102 | assertThat(entry.deviceId(), is(did("id1"))); |
100 | assertThat(entry.selector(), is(SELECTOR)); | 103 | assertThat(entry.selector(), is(SELECTOR)); |
101 | assertThat(entry.treatment(), is(TREATMENT)); | 104 | assertThat(entry.treatment(), is(TREATMENT)); |
102 | assertThat(entry.timeout(), is(1)); | 105 | assertThat(entry.timeout(), is(1)); |
103 | assertThat(entry.life(), is(33L)); | 106 | assertThat(entry.life(), is(33L)); |
107 | + assertThat(entry.life(TimeUnit.MILLISECONDS), is(33333L)); | ||
104 | assertThat(entry.packets(), is(11L)); | 108 | assertThat(entry.packets(), is(11L)); |
105 | assertThat(entry.bytes(), is(22L)); | 109 | assertThat(entry.bytes(), is(22L)); |
106 | assertThat(entry.state(), is(FlowEntry.FlowEntryState.PENDING_REMOVE)); | 110 | assertThat(entry.state(), is(FlowEntry.FlowEntryState.PENDING_REMOVE)); | ... | ... |
... | @@ -51,7 +51,7 @@ public final class FlowEntryCodec extends JsonCodec<FlowEntry> { | ... | @@ -51,7 +51,7 @@ public final class FlowEntryCodec extends JsonCodec<FlowEntry> { |
51 | .put("isPermanent", flowEntry.isPermanent()) | 51 | .put("isPermanent", flowEntry.isPermanent()) |
52 | .put("deviceId", flowEntry.deviceId().toString()) | 52 | .put("deviceId", flowEntry.deviceId().toString()) |
53 | .put("state", flowEntry.state().toString()) | 53 | .put("state", flowEntry.state().toString()) |
54 | - .put("life", flowEntry.life()) | 54 | + .put("life", flowEntry.life()) //FIXME life is destroying precision (seconds granularity is default) |
55 | .put("packets", flowEntry.packets()) | 55 | .put("packets", flowEntry.packets()) |
56 | .put("bytes", flowEntry.bytes()) | 56 | .put("bytes", flowEntry.bytes()) |
57 | .put("lastSeen", flowEntry.lastSeen()); | 57 | .put("lastSeen", flowEntry.lastSeen()); | ... | ... |
... | @@ -569,7 +569,7 @@ public class DistributedFlowRuleStore | ... | @@ -569,7 +569,7 @@ public class DistributedFlowRuleStore |
569 | if (stored != null) { | 569 | if (stored != null) { |
570 | //FIXME modification of "stored" flow entry outside of flow table | 570 | //FIXME modification of "stored" flow entry outside of flow table |
571 | stored.setBytes(rule.bytes()); | 571 | stored.setBytes(rule.bytes()); |
572 | - stored.setLife(rule.life()); | 572 | + stored.setLife(rule.life(TimeUnit.NANOSECONDS), TimeUnit.NANOSECONDS); |
573 | stored.setPackets(rule.packets()); | 573 | stored.setPackets(rule.packets()); |
574 | stored.setLastSeen(); | 574 | stored.setLastSeen(); |
575 | if (stored.state() == FlowEntryState.PENDING_ADD) { | 575 | if (stored.state() == FlowEntryState.PENDING_ADD) { | ... | ... |
... | @@ -449,7 +449,7 @@ public class NewAdaptiveFlowStatsCollector implements SwitchDataCollector { | ... | @@ -449,7 +449,7 @@ public class NewAdaptiveFlowStatsCollector implements SwitchDataCollector { |
449 | 449 | ||
450 | // update now | 450 | // update now |
451 | //FIXME modification of "stored" flow entry outside of store | 451 | //FIXME modification of "stored" flow entry outside of store |
452 | - stored.setLife(fe.life()); | 452 | + stored.setLife(fe.life(TimeUnit.NANOSECONDS), TimeUnit.NANOSECONDS); |
453 | stored.setPackets(fe.packets()); | 453 | stored.setPackets(fe.packets()); |
454 | stored.setBytes(fe.bytes()); | 454 | stored.setBytes(fe.bytes()); |
455 | stored.setLastSeen(); | 455 | stored.setLastSeen(); | ... | ... |
... | @@ -94,6 +94,8 @@ import org.slf4j.LoggerFactory; | ... | @@ -94,6 +94,8 @@ import org.slf4j.LoggerFactory; |
94 | 94 | ||
95 | import java.util.List; | 95 | import java.util.List; |
96 | 96 | ||
97 | +import static java.util.concurrent.TimeUnit.NANOSECONDS; | ||
98 | +import static java.util.concurrent.TimeUnit.SECONDS; | ||
97 | import static org.onosproject.net.flow.criteria.Criteria.*; | 99 | import static org.onosproject.net.flow.criteria.Criteria.*; |
98 | import static org.onosproject.net.flow.instructions.Instructions.modL0Lambda; | 100 | import static org.onosproject.net.flow.instructions.Instructions.modL0Lambda; |
99 | import static org.onosproject.net.flow.instructions.Instructions.modL1OduSignalId; | 101 | import static org.onosproject.net.flow.instructions.Instructions.modL1OduSignalId; |
... | @@ -170,7 +172,8 @@ public class FlowEntryBuilder { | ... | @@ -170,7 +172,8 @@ public class FlowEntryBuilder { |
170 | } | 172 | } |
171 | 173 | ||
172 | return new DefaultFlowEntry(builder.build(), FlowEntryState.ADDED, | 174 | return new DefaultFlowEntry(builder.build(), FlowEntryState.ADDED, |
173 | - stat.getDurationSec(), | 175 | + SECONDS.toNanos(stat.getDurationSec()) |
176 | + + stat.getDurationNsec(), NANOSECONDS, | ||
174 | stat.getPacketCount().getValue(), | 177 | stat.getPacketCount().getValue(), |
175 | stat.getByteCount().getValue()); | 178 | stat.getByteCount().getValue()); |
176 | case REMOVED: | 179 | case REMOVED: |
... | @@ -185,7 +188,8 @@ public class FlowEntryBuilder { | ... | @@ -185,7 +188,8 @@ public class FlowEntryBuilder { |
185 | } | 188 | } |
186 | 189 | ||
187 | return new DefaultFlowEntry(builder.build(), FlowEntryState.REMOVED, | 190 | return new DefaultFlowEntry(builder.build(), FlowEntryState.REMOVED, |
188 | - removed.getDurationSec(), | 191 | + SECONDS.toNanos(removed.getDurationSec()) |
192 | + + removed.getDurationNsec(), NANOSECONDS, | ||
189 | removed.getPacketCount().getValue(), | 193 | removed.getPacketCount().getValue(), |
190 | removed.getByteCount().getValue()); | 194 | removed.getByteCount().getValue()); |
191 | case MOD: | 195 | case MOD: | ... | ... |
... | @@ -64,7 +64,9 @@ import java.net.HttpURLConnection; | ... | @@ -64,7 +64,9 @@ import java.net.HttpURLConnection; |
64 | import java.util.HashMap; | 64 | import java.util.HashMap; |
65 | import java.util.HashSet; | 65 | import java.util.HashSet; |
66 | import java.util.Set; | 66 | import java.util.Set; |
67 | +import java.util.concurrent.TimeUnit; | ||
67 | 68 | ||
69 | +import static java.util.concurrent.TimeUnit.SECONDS; | ||
68 | import static org.easymock.EasyMock.anyObject; | 70 | import static org.easymock.EasyMock.anyObject; |
69 | import static org.easymock.EasyMock.anyShort; | 71 | import static org.easymock.EasyMock.anyShort; |
70 | import static org.easymock.EasyMock.anyString; | 72 | import static org.easymock.EasyMock.anyString; |
... | @@ -141,7 +143,12 @@ public class FlowsResourceTest extends ResourceTest { | ... | @@ -141,7 +143,12 @@ public class FlowsResourceTest extends ResourceTest { |
141 | 143 | ||
142 | @Override | 144 | @Override |
143 | public long life() { | 145 | public long life() { |
144 | - return baseValue + 11; | 146 | + return life(SECONDS); |
147 | + } | ||
148 | + | ||
149 | + @Override | ||
150 | + public long life(TimeUnit timeUnit) { | ||
151 | + return SECONDS.convert(baseValue + 11, timeUnit); | ||
145 | } | 152 | } |
146 | 153 | ||
147 | @Override | 154 | @Override | ... | ... |
-
Please register or login to post a comment