Re: [nox-dev] Handling corrupted TCP header
Hi KK The patch looks right. I will push that to SNAC too. Thanks Srini. On Fri, Jan 14, 2011 at 10:47 AM, kk yap wrote: > Okay, I think the high level point is we should expose malformed > packets and let others decide how to handle it. Can someone review > this patch before I push? > > Regards > KK > > On 14 January 2011 01:25, Rob Sherwood wrote: >> Fwiw, I agree with what both Masa and KK. >> >> Masa's point: OFPP_TABLE shouldn't be a special case: i.e., it should >> be able to generate packet_in's on the second pss >> >> KK's point: this should be more explicitly called out in the spec. >> >> I think if you were to suggest a specific wording in the next week, >> this could still make it into the 1.1 spec. >> >> - Rob >> . >> >> >> >> On Thu, Jan 13, 2011 at 7:06 PM, Masayoshi Kobayashi >> wrote: >>> KK, >>> >>> I think the implementer will read the spec the other way around. >>> Spec requires nothing special about OFPP_TABLE action (it does not say >>> "don't generate pkt_in, if there is no match"). So the switch >>> just follows the default behavior, i.e., pkt_in will be generated. >>> >>> I would expect the reference design also does the same. >>> >>> Masa >>> >>> On 01/13/2011 06:38 PM, kk yap wrote: > > Because the action of pkt_out is "OFPP_TABLE". > (the packet in pkt_out does not match to the entry that is installed by > flow_mod, since the matching entry says nw_proto=0). Is there anything in the spec that says that the switch should send another packet-in if there is no matching entry for a OFPP_TABLE? I am failing to find that. Can someone point to why this behavior is specified by the spec? Regards KK On 13 January 2011 18:28, Masayoshi Kobayashi wrote: > > KK, > >> I thought about it a little. Why is the switch doing step 7? > > Because the action of pkt_out is "OFPP_TABLE". > (the packet in pkt_out does not match to the entry that is installed by > flow_mod, since the matching entry says nw_proto=0). > > Masa > > On 01/13/2011 06:06 PM, kk yap wrote: >> >> Hi Srini, >> >> I thought about it a little. Why is the switch doing step 7? >> >> Anyway, I do agree that NOX is not handling malformed packets right. >> I have included an invalid field in the struct flow, and created a >> component that ignore invalid packets. If anyone will double-check >> and test the patches attached, I will push it. >> >> Regards >> KK >> >> On 13 January 2011 16:13, Srini Seetharaman >> wrote: >>> >>> I explained this to KK in person. For others, here is the sequence of >>> events: >>> >>> 1. Packet arrives with (nw_proto=6, tp_src=0, tp_dst=0). Store in bufid >>> 'X' >>> 2. flow.cc identifies that the arrived TCP packet is corrupted, and >>> generates >>> pkt_in event with flow structure having (nw_proto=0, tp_src=0, >>> tp_dst=0) >>> 3. Authenticator generates a flow_in with flow_in.flow being same as >>> above >>> 3. routing.cc generates a flow_mod for the flow_in with the match >>> pattern >>> defined using the fields of the flow_in.flow >>> 4. Switch inserts a flow table entry for matching (nw_proto=0, >>> tp_src=0, tp_dst=0) >>> 5. routing.cc generates a pkt_out for the bufid 'X' with action = >>> OFPP_TABLE >>> 6. Switch notices that the packet in bufid 'X' has no matching flow >>> table >>> entry, >>> because there is a mismatch on the nw_proto field >>> 7. Switch generates a new pkt_in event >>> 8. Go to step (2) >>> >>> This is the infinite loop. >>> >>> Srini. >>> >>> On Thu, Jan 13, 2011 at 1:08 PM, kk yap wrote: Hi Srini, I think you are fixing this in the wrong place. Putting nw_proto=0 does not cause an infinite loop. Where is the loop happening? Can you provide more detailed NOX output so that we can even start looking at this. Regards KK On 13 January 2011 11:02, Srini Seetharaman wrote: > > We don't know who sent it, but it came from outside our network. If > it > is easy to take down a network by just sending 1 invalid packet, I'd > be worried! > > On Thu, Jan 13, 2011 at 10:59 AM, kk yap > wrote: >> >> Hi Srini, >> >> What is this packet? The length of TCP is zero?!?! I wish to >> understand the circumstance for which we are getting the packet >> before >> commenting on the right way to handle this. >> >> Regards >> KK >> >> >> On 13 January 2011 10:38, Srini Seetharaman >> wrote: >>> >>> When someone sends the attached packet to a switch, it generates an >>
Re: [nox-dev] pytopology patch
Please use git-format-patch. Regards KK On 14 January 2011 10:52, Nikhil Handigol wrote: > I haven't used git-am. What format does it expect the patch to be in? > > I usually do the following: > $ cd nox > $ patch -p1 < path/to/patch/pytopology_destiny.patch > > -/\/ > > > On Fri, Jan 14, 2011 at 10:39 AM, kk yap wrote: >> >> Hi Nikhil, >> >> That did not work. >> >> ykk@kk-alien:~$ git am pytopology_destiny.patch >> Patch format detection failed. >> >> Regards >> KK >> >> On 13 January 2011 20:44, Martin Casado wrote: >> > Awesome, thanks Nikhil. >> > >> > Hi, >> > I have added 2 new functions to pytopology that makes it more >> > useful/usable: >> > * get_datapaths() : returns a list of all datapathids in the network >> > * get_neighbors(dpid) : returns a list of neighbors of a given >> > datapathid. >> > I've attached a patch for the destiny branch (commit >> > no. 5917f412d6d2a23ce5373ec499872cb8b40833e8). >> > Thanks, >> > Nikhil >> > >> > ___ >> > nox-dev mailing list >> > nox-dev@noxrepo.org >> > http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org >> > >> > >> > -- >> > ~~~ >> > Martin Casado >> > Nicira Networks, Inc. >> > www.nicira.com | www.openvswitch.org >> > cell: 650-776-1457 >> > ~~~ >> > >> > ___ >> > nox-dev mailing list >> > nox-dev@noxrepo.org >> > http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org >> > >> > > > ___ nox-dev mailing list nox-dev@noxrepo.org http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org
Re: [nox-dev] pytopology patch
I haven't used git-am. What format does it expect the patch to be in? I usually do the following: $ cd nox $ patch -p1 < path/to/patch/pytopology_destiny.patch -/\/ On Fri, Jan 14, 2011 at 10:39 AM, kk yap wrote: > Hi Nikhil, > > That did not work. > > ykk@kk-alien:~$ git am pytopology_destiny.patch > Patch format detection failed. > > Regards > KK > > On 13 January 2011 20:44, Martin Casado wrote: > > Awesome, thanks Nikhil. > > > > Hi, > > I have added 2 new functions to pytopology that makes it more > useful/usable: > > * get_datapaths() : returns a list of all datapathids in the network > > * get_neighbors(dpid) : returns a list of neighbors of a given > datapathid. > > I've attached a patch for the destiny branch (commit > > no. 5917f412d6d2a23ce5373ec499872cb8b40833e8). > > Thanks, > > Nikhil > > > > ___ > > nox-dev mailing list > > nox-dev@noxrepo.org > > http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org > > > > > > -- > > ~~~ > > Martin Casado > > Nicira Networks, Inc. > > www.nicira.com | www.openvswitch.org > > cell: 650-776-1457 > > ~~~ > > > > ___ > > nox-dev mailing list > > nox-dev@noxrepo.org > > http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org > > > > > ___ nox-dev mailing list nox-dev@noxrepo.org http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org
Re: [nox-dev] Handling corrupted TCP header
Okay, I think the high level point is we should expose malformed packets and let others decide how to handle it. Can someone review this patch before I push? Regards KK On 14 January 2011 01:25, Rob Sherwood wrote: > Fwiw, I agree with what both Masa and KK. > > Masa's point: OFPP_TABLE shouldn't be a special case: i.e., it should > be able to generate packet_in's on the second pss > > KK's point: this should be more explicitly called out in the spec. > > I think if you were to suggest a specific wording in the next week, > this could still make it into the 1.1 spec. > > - Rob > . > > > > On Thu, Jan 13, 2011 at 7:06 PM, Masayoshi Kobayashi > wrote: >> KK, >> >> I think the implementer will read the spec the other way around. >> Spec requires nothing special about OFPP_TABLE action (it does not say >> "don't generate pkt_in, if there is no match"). So the switch >> just follows the default behavior, i.e., pkt_in will be generated. >> >> I would expect the reference design also does the same. >> >> Masa >> >> On 01/13/2011 06:38 PM, kk yap wrote: Because the action of pkt_out is "OFPP_TABLE". (the packet in pkt_out does not match to the entry that is installed by flow_mod, since the matching entry says nw_proto=0). >>> >>> Is there anything in the spec that says that the switch should send >>> another packet-in if there is no matching entry for a OFPP_TABLE? I >>> am failing to find that. Can someone point to why this behavior is >>> specified by the spec? >>> >>> Regards >>> KK >>> >>> On 13 January 2011 18:28, Masayoshi Kobayashi >>> wrote: KK, > I thought about it a little. Why is the switch doing step 7? Because the action of pkt_out is "OFPP_TABLE". (the packet in pkt_out does not match to the entry that is installed by flow_mod, since the matching entry says nw_proto=0). Masa On 01/13/2011 06:06 PM, kk yap wrote: > > Hi Srini, > > I thought about it a little. Why is the switch doing step 7? > > Anyway, I do agree that NOX is not handling malformed packets right. > I have included an invalid field in the struct flow, and created a > component that ignore invalid packets. If anyone will double-check > and test the patches attached, I will push it. > > Regards > KK > > On 13 January 2011 16:13, Srini Seetharaman > wrote: >> >> I explained this to KK in person. For others, here is the sequence of >> events: >> >> 1. Packet arrives with (nw_proto=6, tp_src=0, tp_dst=0). Store in bufid >> 'X' >> 2. flow.cc identifies that the arrived TCP packet is corrupted, and >> generates >> pkt_in event with flow structure having (nw_proto=0, tp_src=0, >> tp_dst=0) >> 3. Authenticator generates a flow_in with flow_in.flow being same as >> above >> 3. routing.cc generates a flow_mod for the flow_in with the match >> pattern >> defined using the fields of the flow_in.flow >> 4. Switch inserts a flow table entry for matching (nw_proto=0, >> tp_src=0, tp_dst=0) >> 5. routing.cc generates a pkt_out for the bufid 'X' with action = >> OFPP_TABLE >> 6. Switch notices that the packet in bufid 'X' has no matching flow >> table >> entry, >> because there is a mismatch on the nw_proto field >> 7. Switch generates a new pkt_in event >> 8. Go to step (2) >> >> This is the infinite loop. >> >> Srini. >> >> On Thu, Jan 13, 2011 at 1:08 PM, kk yap wrote: >>> >>> Hi Srini, >>> >>> I think you are fixing this in the wrong place. Putting nw_proto=0 >>> does not cause an infinite loop. Where is the loop happening? Can >>> you provide more detailed NOX output so that we can even start looking >>> at this. >>> >>> Regards >>> KK >>> >>> On 13 January 2011 11:02, Srini Seetharaman >>> wrote: We don't know who sent it, but it came from outside our network. If it is easy to take down a network by just sending 1 invalid packet, I'd be worried! On Thu, Jan 13, 2011 at 10:59 AM, kk yap wrote: > > Hi Srini, > > What is this packet? The length of TCP is zero?!?! I wish to > understand the circumstance for which we are getting the packet > before > commenting on the right way to handle this. > > Regards > KK > > > On 13 January 2011 10:38, Srini Seetharaman > wrote: >> >> When someone sends the attached packet to a switch, it generates an >> infinite loop of packet_ins in our production network. This is >> because >> this incoming tcp packet has nw_proto=6 and tcp port numbers of >> "0", >> but outgoing flow_mod has nw_proto of "0" and tcp port numbers of >> "0". >>
Re: [nox-dev] pytopology patch
Hi Nikhil, That did not work. ykk@kk-alien:~$ git am pytopology_destiny.patch Patch format detection failed. Regards KK On 13 January 2011 20:44, Martin Casado wrote: > Awesome, thanks Nikhil. > > Hi, > I have added 2 new functions to pytopology that makes it more useful/usable: > * get_datapaths() : returns a list of all datapathids in the network > * get_neighbors(dpid) : returns a list of neighbors of a given datapathid. > I've attached a patch for the destiny branch (commit > no. 5917f412d6d2a23ce5373ec499872cb8b40833e8). > Thanks, > Nikhil > > ___ > nox-dev mailing list > nox-dev@noxrepo.org > http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org > > > -- > ~~~ > Martin Casado > Nicira Networks, Inc. > www.nicira.com | www.openvswitch.org > cell: 650-776-1457 > ~~~ > > ___ > nox-dev mailing list > nox-dev@noxrepo.org > http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org > > ___ nox-dev mailing list nox-dev@noxrepo.org http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org
Re: [nox-dev] Handling corrupted TCP header
Fwiw, I agree with what both Masa and KK. Masa's point: OFPP_TABLE shouldn't be a special case: i.e., it should be able to generate packet_in's on the second pss KK's point: this should be more explicitly called out in the spec. I think if you were to suggest a specific wording in the next week, this could still make it into the 1.1 spec. - Rob . On Thu, Jan 13, 2011 at 7:06 PM, Masayoshi Kobayashi wrote: > KK, > > I think the implementer will read the spec the other way around. > Spec requires nothing special about OFPP_TABLE action (it does not say > "don't generate pkt_in, if there is no match"). So the switch > just follows the default behavior, i.e., pkt_in will be generated. > > I would expect the reference design also does the same. > > Masa > > On 01/13/2011 06:38 PM, kk yap wrote: >>> >>> Because the action of pkt_out is "OFPP_TABLE". >>> (the packet in pkt_out does not match to the entry that is installed by >>> flow_mod, since the matching entry says nw_proto=0). >> >> Is there anything in the spec that says that the switch should send >> another packet-in if there is no matching entry for a OFPP_TABLE? I >> am failing to find that. Can someone point to why this behavior is >> specified by the spec? >> >> Regards >> KK >> >> On 13 January 2011 18:28, Masayoshi Kobayashi >> wrote: >>> >>> KK, >>> I thought about it a little. Why is the switch doing step 7? >>> >>> Because the action of pkt_out is "OFPP_TABLE". >>> (the packet in pkt_out does not match to the entry that is installed by >>> flow_mod, since the matching entry says nw_proto=0). >>> >>> Masa >>> >>> On 01/13/2011 06:06 PM, kk yap wrote: Hi Srini, I thought about it a little. Why is the switch doing step 7? Anyway, I do agree that NOX is not handling malformed packets right. I have included an invalid field in the struct flow, and created a component that ignore invalid packets. If anyone will double-check and test the patches attached, I will push it. Regards KK On 13 January 2011 16:13, Srini Seetharaman wrote: > > I explained this to KK in person. For others, here is the sequence of > events: > > 1. Packet arrives with (nw_proto=6, tp_src=0, tp_dst=0). Store in bufid > 'X' > 2. flow.cc identifies that the arrived TCP packet is corrupted, and > generates > pkt_in event with flow structure having (nw_proto=0, tp_src=0, > tp_dst=0) > 3. Authenticator generates a flow_in with flow_in.flow being same as > above > 3. routing.cc generates a flow_mod for the flow_in with the match > pattern > defined using the fields of the flow_in.flow > 4. Switch inserts a flow table entry for matching (nw_proto=0, > tp_src=0, tp_dst=0) > 5. routing.cc generates a pkt_out for the bufid 'X' with action = > OFPP_TABLE > 6. Switch notices that the packet in bufid 'X' has no matching flow > table > entry, > because there is a mismatch on the nw_proto field > 7. Switch generates a new pkt_in event > 8. Go to step (2) > > This is the infinite loop. > > Srini. > > On Thu, Jan 13, 2011 at 1:08 PM, kk yap wrote: >> >> Hi Srini, >> >> I think you are fixing this in the wrong place. Putting nw_proto=0 >> does not cause an infinite loop. Where is the loop happening? Can >> you provide more detailed NOX output so that we can even start looking >> at this. >> >> Regards >> KK >> >> On 13 January 2011 11:02, Srini Seetharaman >> wrote: >>> >>> We don't know who sent it, but it came from outside our network. If >>> it >>> is easy to take down a network by just sending 1 invalid packet, I'd >>> be worried! >>> >>> On Thu, Jan 13, 2011 at 10:59 AM, kk yap >>> wrote: Hi Srini, What is this packet? The length of TCP is zero?!?! I wish to understand the circumstance for which we are getting the packet before commenting on the right way to handle this. Regards KK On 13 January 2011 10:38, Srini Seetharaman wrote: > > When someone sends the attached packet to a switch, it generates an > infinite loop of packet_ins in our production network. This is > because > this incoming tcp packet has nw_proto=6 and tcp port numbers of > "0", > but outgoing flow_mod has nw_proto of "0" and tcp port numbers of > "0". > So, the packet_out generates a new packet_in and this loop > continues > forever. > > I see the following code in src/lib/flow.cc (both in NOX-Zaku and > SNAC). I believe this is what is causing the nw_proto to be "0" in > the > flow_mod. I'm not sure who wrote that piece of code. This is not > handl
