I think someone should clarify the generation of packet-in due to OFPP_TABLE on opeflow-spec.
Also, I am not sure why you want to forward a malformed packet, but that should be a simple change to the packetcheck module. I think I would prefer to drop senseless packets in the network. Supporting legacy bad behavior is not very high on my list of priorities. I think the change in the flow structure is needed in NOX. I am happy to drop packetcheck component, which I agree make too much assumption. And the way to handle malformed packet can be deployment specific, i.e., the flow structure tell you it is malformed and you have a component to do what you want. Hope I am making sense. Regards KK On 13 January 2011 19:06, Masayoshi Kobayashi <[email protected]> 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<[email protected]> >> 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<[email protected]> >>>> 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<[email protected]> 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<[email protected]> >>>>>> 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<[email protected]> >>>>>>> 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<[email protected]> >>>>>>>> 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 >>>>>>>>> handling corrupted packets well and rejecting this packet as a >>>>>>>>> invalid >>>>>>>>> TCP packet. Does anyone see problems with removing the "else" >>>>>>>>> clause? >>>>>>>>> >>>>>>>>> if (nw_proto == ip_::proto::TCP) { >>>>>>>>> const tcp_header *tcp = pull_tcp(b); >>>>>>>>> if (tcp) { >>>>>>>>> tp_src = tcp->tcp_src; >>>>>>>>> tp_dst = tcp->tcp_dst; >>>>>>>>> } else { >>>>>>>>> /* Avoid tricking other code into thinking that >>>>>>>>> * this packet has an L4 header. */ >>>>>>>>> nw_proto = 0; >>>>>>>>> } >>>>>>>>> } >>>>>>>>> >>>>>>>>> FYI, pull_tcp is defined as below: >>>>>>>>> static const tcp_header * pull_tcp(Buffer& b) >>>>>>>>> { >>>>>>>>> if (const tcp_header *tcp = b.try_at<tcp_header>(0)) { >>>>>>>>> int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4; >>>>>>>>> if (tcp_len>= sizeof *tcp) { >>>>>>>>> return reinterpret_cast<const >>>>>>>>> tcp_header*>(b.try_pull(tcp_len)); >>>>>>>>> } >>>>>>>>> } >>>>>>>>> return 0; >>>>>>>>> } >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> nox-dev mailing list >>>>>>>>> [email protected] >>>>>>>>> http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> nox-dev mailing list >>>>> [email protected] >>>>> http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org >>> >>> -- >>> Masayoshi Kobayashi >>> Visiting Scholar at Stanford University >>> System Platform Research Labs, NEC Corporation. >>> T/M: 650-714-3154 >>> >>> _______________________________________________ >>> nox-dev mailing list >>> [email protected] >>> http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org >>> > > -- > Masayoshi Kobayashi > Visiting Scholar at Stanford University > System Platform Research Labs, NEC Corporation. > T/M: 650-714-3154 > _______________________________________________ nox-dev mailing list [email protected] http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org
