Committed by
Gerrit Code Review
BGP encode multiple flow spec action issue fix.
Change-Id: Ieb173be9edb86bc717bfe1e55271873645e4a5b1
Showing
4 changed files
with
75 additions
and
40 deletions
| ... | @@ -27,7 +27,7 @@ import com.google.common.base.MoreObjects; | ... | @@ -27,7 +27,7 @@ import com.google.common.base.MoreObjects; |
| 27 | */ | 27 | */ |
| 28 | public class BgpFlowSpecDetails { | 28 | public class BgpFlowSpecDetails { |
| 29 | private List<BgpValueType> flowSpecComponents; | 29 | private List<BgpValueType> flowSpecComponents; |
| 30 | - private BgpValueType fsActionTlv; | 30 | + private List<BgpValueType> fsActionTlv; |
| 31 | private RouteDistinguisher routeDistinguisher; | 31 | private RouteDistinguisher routeDistinguisher; |
| 32 | 32 | ||
| 33 | /** | 33 | /** |
| ... | @@ -44,7 +44,7 @@ public class BgpFlowSpecDetails { | ... | @@ -44,7 +44,7 @@ public class BgpFlowSpecDetails { |
| 44 | * | 44 | * |
| 45 | * @return flow specification action tlv | 45 | * @return flow specification action tlv |
| 46 | */ | 46 | */ |
| 47 | - public BgpValueType fsActionTlv() { | 47 | + public List<BgpValueType> fsActionTlv() { |
| 48 | return this.fsActionTlv; | 48 | return this.fsActionTlv; |
| 49 | } | 49 | } |
| 50 | 50 | ||
| ... | @@ -53,7 +53,7 @@ public class BgpFlowSpecDetails { | ... | @@ -53,7 +53,7 @@ public class BgpFlowSpecDetails { |
| 53 | * | 53 | * |
| 54 | * @param fsActionTlv flow specification action tlv | 54 | * @param fsActionTlv flow specification action tlv |
| 55 | */ | 55 | */ |
| 56 | - public void setFsActionTlv(BgpValueType fsActionTlv) { | 56 | + public void setFsActionTlv(List<BgpValueType> fsActionTlv) { |
| 57 | this.fsActionTlv = fsActionTlv; | 57 | this.fsActionTlv = fsActionTlv; |
| 58 | } | 58 | } |
| 59 | 59 | ... | ... |
| ... | @@ -23,6 +23,10 @@ import org.onosproject.bgpio.util.Constants; | ... | @@ -23,6 +23,10 @@ import org.onosproject.bgpio.util.Constants; |
| 23 | import org.onosproject.bgpio.util.Validation; | 23 | import org.onosproject.bgpio.util.Validation; |
| 24 | import org.slf4j.Logger; | 24 | import org.slf4j.Logger; |
| 25 | import org.slf4j.LoggerFactory; | 25 | import org.slf4j.LoggerFactory; |
| 26 | + | ||
| 27 | +import java.util.LinkedList; | ||
| 28 | +import java.util.List; | ||
| 29 | +import java.util.ListIterator; | ||
| 26 | import java.util.Objects; | 30 | import java.util.Objects; |
| 27 | 31 | ||
| 28 | /** | 32 | /** |
| ... | @@ -33,14 +37,14 @@ public class BgpExtendedCommunity implements BgpValueType { | ... | @@ -33,14 +37,14 @@ public class BgpExtendedCommunity implements BgpValueType { |
| 33 | private static final Logger log = LoggerFactory.getLogger(BgpExtendedCommunity.class); | 37 | private static final Logger log = LoggerFactory.getLogger(BgpExtendedCommunity.class); |
| 34 | public static final short TYPE = Constants.BGP_EXTENDED_COMMUNITY; | 38 | public static final short TYPE = Constants.BGP_EXTENDED_COMMUNITY; |
| 35 | public static final byte FLAGS = (byte) 0xC0; | 39 | public static final byte FLAGS = (byte) 0xC0; |
| 36 | - private BgpValueType fsActionTlv; | 40 | + private List<BgpValueType> fsActionTlv; |
| 37 | 41 | ||
| 38 | /** | 42 | /** |
| 39 | * Constructor to initialize the value. | 43 | * Constructor to initialize the value. |
| 40 | * | 44 | * |
| 41 | * @param fsActionTlv flow specification action type | 45 | * @param fsActionTlv flow specification action type |
| 42 | */ | 46 | */ |
| 43 | - public BgpExtendedCommunity(BgpValueType fsActionTlv) { | 47 | + public BgpExtendedCommunity(List<BgpValueType> fsActionTlv) { |
| 44 | this.fsActionTlv = fsActionTlv; | 48 | this.fsActionTlv = fsActionTlv; |
| 45 | } | 49 | } |
| 46 | 50 | ||
| ... | @@ -49,7 +53,7 @@ public class BgpExtendedCommunity implements BgpValueType { | ... | @@ -49,7 +53,7 @@ public class BgpExtendedCommunity implements BgpValueType { |
| 49 | * | 53 | * |
| 50 | * @return extended community | 54 | * @return extended community |
| 51 | */ | 55 | */ |
| 52 | - public BgpValueType fsActionTlv() { | 56 | + public List<BgpValueType> fsActionTlv() { |
| 53 | return this.fsActionTlv; | 57 | return this.fsActionTlv; |
| 54 | } | 58 | } |
| 55 | 59 | ||
| ... | @@ -64,7 +68,7 @@ public class BgpExtendedCommunity implements BgpValueType { | ... | @@ -64,7 +68,7 @@ public class BgpExtendedCommunity implements BgpValueType { |
| 64 | 68 | ||
| 65 | ChannelBuffer tempCb = cb.copy(); | 69 | ChannelBuffer tempCb = cb.copy(); |
| 66 | Validation validation = Validation.parseAttributeHeader(cb); | 70 | Validation validation = Validation.parseAttributeHeader(cb); |
| 67 | - BgpValueType fsActionTlv = null; | 71 | + List<BgpValueType> fsActionTlvs = new LinkedList<>(); |
| 68 | 72 | ||
| 69 | if (cb.readableBytes() < validation.getLength()) { | 73 | if (cb.readableBytes() < validation.getLength()) { |
| 70 | Validation.validateLen(BgpErrorType.UPDATE_MESSAGE_ERROR, BgpErrorType.ATTRIBUTE_LENGTH_ERROR, | 74 | Validation.validateLen(BgpErrorType.UPDATE_MESSAGE_ERROR, BgpErrorType.ATTRIBUTE_LENGTH_ERROR, |
| ... | @@ -80,27 +84,33 @@ public class BgpExtendedCommunity implements BgpValueType { | ... | @@ -80,27 +84,33 @@ public class BgpExtendedCommunity implements BgpValueType { |
| 80 | 84 | ||
| 81 | ChannelBuffer tempBuf = cb.readBytes(validation.getLength()); | 85 | ChannelBuffer tempBuf = cb.readBytes(validation.getLength()); |
| 82 | if (tempBuf.readableBytes() > 0) { | 86 | if (tempBuf.readableBytes() > 0) { |
| 87 | + BgpValueType fsActionTlv = null; | ||
| 83 | ChannelBuffer actionBuf = tempBuf.readBytes(validation.getLength()); | 88 | ChannelBuffer actionBuf = tempBuf.readBytes(validation.getLength()); |
| 84 | - short actionType = actionBuf.readShort(); | 89 | + |
| 85 | - short length = (short) validation.getLength(); | 90 | + while (actionBuf.readableBytes() > 0) { |
| 86 | - switch (actionType) { | 91 | + short actionType = actionBuf.readShort(); |
| 87 | - case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_ACTION: | 92 | + switch (actionType) { |
| 88 | - fsActionTlv = BgpFsActionTrafficAction.read(actionBuf); | 93 | + case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_ACTION: |
| 89 | - break; | 94 | + fsActionTlv = BgpFsActionTrafficAction.read(actionBuf); |
| 90 | - case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_MARKING: | 95 | + break; |
| 91 | - fsActionTlv = BgpFsActionTrafficMarking.read(actionBuf); | 96 | + case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_MARKING: |
| 92 | - break; | 97 | + fsActionTlv = BgpFsActionTrafficMarking.read(actionBuf); |
| 93 | - case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_RATE: | 98 | + break; |
| 94 | - fsActionTlv = BgpFsActionTrafficRate.read(actionBuf); | 99 | + case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_RATE: |
| 95 | - break; | 100 | + fsActionTlv = BgpFsActionTrafficRate.read(actionBuf); |
| 96 | - case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_REDIRECT: | 101 | + break; |
| 97 | - fsActionTlv = BgpFsActionReDirect.read(actionBuf); | 102 | + case Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_REDIRECT: |
| 98 | - break; | 103 | + fsActionTlv = BgpFsActionReDirect.read(actionBuf); |
| 99 | - default: log.debug("Other type Not Supported:" + actionType); | 104 | + break; |
| 100 | - break; | 105 | + default: log.debug("Other type Not Supported:" + actionType); |
| 106 | + break; | ||
| 107 | + } | ||
| 108 | + } | ||
| 109 | + if (fsActionTlv != null) { | ||
| 110 | + fsActionTlvs.add(fsActionTlv); | ||
| 101 | } | 111 | } |
| 102 | } | 112 | } |
| 103 | - return new BgpExtendedCommunity(fsActionTlv); | 113 | + return new BgpExtendedCommunity(fsActionTlvs); |
| 104 | } | 114 | } |
| 105 | 115 | ||
| 106 | @Override | 116 | @Override |
| ... | @@ -136,24 +146,29 @@ public class BgpExtendedCommunity implements BgpValueType { | ... | @@ -136,24 +146,29 @@ public class BgpExtendedCommunity implements BgpValueType { |
| 136 | @Override | 146 | @Override |
| 137 | public int write(ChannelBuffer cb) { | 147 | public int write(ChannelBuffer cb) { |
| 138 | int iLenStartIndex = cb.writerIndex(); | 148 | int iLenStartIndex = cb.writerIndex(); |
| 149 | + ListIterator<BgpValueType> listIterator = fsActionTlv().listIterator(); | ||
| 150 | + | ||
| 139 | cb.writeByte(FLAGS); | 151 | cb.writeByte(FLAGS); |
| 140 | cb.writeByte(getType()); | 152 | cb.writeByte(getType()); |
| 141 | 153 | ||
| 142 | int iActionLenIndex = cb.writerIndex(); | 154 | int iActionLenIndex = cb.writerIndex(); |
| 143 | cb.writeByte(0); | 155 | cb.writeByte(0); |
| 144 | 156 | ||
| 145 | - if (fsActionTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_ACTION) { | 157 | + while (listIterator.hasNext()) { |
| 146 | - BgpFsActionTrafficAction trafficAction = (BgpFsActionTrafficAction) fsActionTlv; | 158 | + BgpValueType fsTlv = listIterator.next(); |
| 147 | - trafficAction.write(cb); | 159 | + if (fsTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_ACTION) { |
| 148 | - } else if (fsActionTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_MARKING) { | 160 | + BgpFsActionTrafficAction trafficAction = (BgpFsActionTrafficAction) fsTlv; |
| 149 | - BgpFsActionTrafficMarking trafficMarking = (BgpFsActionTrafficMarking) fsActionTlv; | 161 | + trafficAction.write(cb); |
| 150 | - trafficMarking.write(cb); | 162 | + } else if (fsTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_MARKING) { |
| 151 | - } else if (fsActionTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_RATE) { | 163 | + BgpFsActionTrafficMarking trafficMarking = (BgpFsActionTrafficMarking) fsTlv; |
| 152 | - BgpFsActionTrafficRate trafficRate = (BgpFsActionTrafficRate) fsActionTlv; | 164 | + trafficMarking.write(cb); |
| 153 | - trafficRate.write(cb); | 165 | + } else if (fsTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_RATE) { |
| 154 | - } else if (fsActionTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_REDIRECT) { | 166 | + BgpFsActionTrafficRate trafficRate = (BgpFsActionTrafficRate) fsTlv; |
| 155 | - BgpFsActionReDirect trafficRedirect = (BgpFsActionReDirect) fsActionTlv; | 167 | + trafficRate.write(cb); |
| 156 | - trafficRedirect.write(cb); | 168 | + } else if (fsTlv.getType() == Constants.BGP_FLOWSPEC_ACTION_TRAFFIC_REDIRECT) { |
| 169 | + BgpFsActionReDirect trafficRedirect = (BgpFsActionReDirect) fsTlv; | ||
| 170 | + trafficRedirect.write(cb); | ||
| 171 | + } | ||
| 157 | } | 172 | } |
| 158 | 173 | ||
| 159 | int fsActionLen = cb.writerIndex() - iActionLenIndex; | 174 | int fsActionLen = cb.writerIndex() - iActionLenIndex; | ... | ... |
| ... | @@ -54,21 +54,29 @@ public class BgpFlowSpecDetailsTest { | ... | @@ -54,21 +54,29 @@ public class BgpFlowSpecDetailsTest { |
| 54 | operatorValue1.add(new BgpFsOperatorValue((byte) 0x81, port1)); | 54 | operatorValue1.add(new BgpFsOperatorValue((byte) 0x81, port1)); |
| 55 | byte[] port11 = new byte[] {(byte) 0x1 }; | 55 | byte[] port11 = new byte[] {(byte) 0x1 }; |
| 56 | operatorValue1.add(new BgpFsOperatorValue((byte) 0x82, port11)); | 56 | operatorValue1.add(new BgpFsOperatorValue((byte) 0x82, port11)); |
| 57 | - flowSpecDetails1.setFsActionTlv(new BgpFsActionTrafficRate((short) 1, 1)); | 57 | + |
| 58 | + List<BgpValueType> fsTlvs1 = new LinkedList<>(); | ||
| 59 | + fsTlvs1.add(new BgpFsActionTrafficRate((short) 1, 1)); | ||
| 60 | + flowSpecDetails1.setFsActionTlv(fsTlvs1); | ||
| 58 | 61 | ||
| 59 | flowSpecComponents.add(portNum); | 62 | flowSpecComponents.add(portNum); |
| 60 | byte[] port = new byte[] {(byte) 0x1 }; | 63 | byte[] port = new byte[] {(byte) 0x1 }; |
| 61 | operatorValue.add(new BgpFsOperatorValue((byte) 0x81, port)); | 64 | operatorValue.add(new BgpFsOperatorValue((byte) 0x81, port)); |
| 62 | byte[] port4 = new byte[] {(byte) 0x1 }; | 65 | byte[] port4 = new byte[] {(byte) 0x1 }; |
| 63 | operatorValue.add(new BgpFsOperatorValue((byte) 0x82, port4)); | 66 | operatorValue.add(new BgpFsOperatorValue((byte) 0x82, port4)); |
| 64 | - flowSpecDetails.setFsActionTlv(new BgpFsActionTrafficRate((short) 1, 1)); | 67 | + |
| 68 | + List<BgpValueType> fsTlvs = new LinkedList<>(); | ||
| 69 | + fsTlvs.add(new BgpFsActionTrafficRate((short) 1, 1)); | ||
| 65 | 70 | ||
| 66 | flowSpecComponents2.add(portNum2); | 71 | flowSpecComponents2.add(portNum2); |
| 67 | byte[] port2 = new byte[] {(byte) 0x1 }; | 72 | byte[] port2 = new byte[] {(byte) 0x1 }; |
| 68 | operatorValue2.add(new BgpFsOperatorValue((byte) 0x82, port2)); | 73 | operatorValue2.add(new BgpFsOperatorValue((byte) 0x82, port2)); |
| 69 | byte[] port22 = new byte[] {(byte) 0x1 }; | 74 | byte[] port22 = new byte[] {(byte) 0x1 }; |
| 70 | operatorValue2.add(new BgpFsOperatorValue((byte) 0x82, port22)); | 75 | operatorValue2.add(new BgpFsOperatorValue((byte) 0x82, port22)); |
| 71 | - flowSpecDetails2.setFsActionTlv(new BgpFsActionTrafficRate((short) 1, 1)); | 76 | + |
| 77 | + List<BgpValueType> fsTlvs2 = new LinkedList<>(); | ||
| 78 | + fsTlvs2.add(new BgpFsActionTrafficRate((short) 1, 1)); | ||
| 79 | + flowSpecDetails2.setFsActionTlv(fsTlvs2); | ||
| 72 | 80 | ||
| 73 | new EqualsTester() | 81 | new EqualsTester() |
| 74 | .addEqualityGroup(flowSpecDetails1, flowSpecDetails) | 82 | .addEqualityGroup(flowSpecDetails1, flowSpecDetails) | ... | ... |
| ... | @@ -252,8 +252,20 @@ public class BgpPeerImpl implements BgpPeer { | ... | @@ -252,8 +252,20 @@ public class BgpPeerImpl implements BgpPeer { |
| 252 | 252 | ||
| 253 | if (operType == FlowSpecOperation.ADD) { | 253 | if (operType == FlowSpecOperation.ADD) { |
| 254 | if (flowSpec.routeDistinguisher() == null) { | 254 | if (flowSpec.routeDistinguisher() == null) { |
| 255 | + if (flowSpecRibOut.flowSpecTree().containsKey(prefix)) { | ||
| 256 | + sendFlowSpecUpdateMessageToPeer(FlowSpecOperation.DELETE, | ||
| 257 | + flowSpecRibOut.flowSpecTree().get(prefix)); | ||
| 258 | + } | ||
| 255 | flowSpecRibOut.add(prefix, flowSpec); | 259 | flowSpecRibOut.add(prefix, flowSpec); |
| 256 | } else { | 260 | } else { |
| 261 | + if (vpnFlowSpecRibOut.vpnFlowSpecTree().containsKey(flowSpec.routeDistinguisher())) { | ||
| 262 | + Map<BgpFlowSpecPrefix, BgpFlowSpecDetails> fsTree; | ||
| 263 | + fsTree = vpnFlowSpecRibOut.vpnFlowSpecTree().get(flowSpec.routeDistinguisher()); | ||
| 264 | + if (fsTree.containsKey(prefix)) { | ||
| 265 | + sendFlowSpecUpdateMessageToPeer(FlowSpecOperation.DELETE, | ||
| 266 | + fsTree.get(prefix)); | ||
| 267 | + } | ||
| 268 | + } | ||
| 257 | vpnFlowSpecRibOut.add(flowSpec.routeDistinguisher(), prefix, flowSpec); | 269 | vpnFlowSpecRibOut.add(flowSpec.routeDistinguisher(), prefix, flowSpec); |
| 258 | } | 270 | } |
| 259 | } else if (operType == FlowSpecOperation.DELETE) { | 271 | } else if (operType == FlowSpecOperation.DELETE) { | ... | ... |
-
Please register or login to post a comment