Re: [nox-dev] Handling corrupted TCP header

2011-01-14 Thread Rob Sherwood
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
mkoba...@stanford.edu 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 Kobayashimkoba...@stanford.edu
  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 Seetharamanseeth...@stanford.edu
  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 yapyap...@stanford.edu    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 Seetharamanseeth...@stanford.edu
  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 yapyap...@stanford.edu
  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 Seetharamanseeth...@stanford.edu
  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)
   

Re: [nox-dev] Handling corrupted TCP header

2011-01-14 Thread kk yap
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 rob.sherw...@stanford.edu 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
 mkoba...@stanford.edu 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 Kobayashimkoba...@stanford.edu
  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 Seetharamanseeth...@stanford.edu
  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 yapyap...@stanford.edu    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 Seetharamanseeth...@stanford.edu
  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 yapyap...@stanford.edu
  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 Seetharamanseeth...@stanford.edu
  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;
     

Re: [nox-dev] Handling corrupted TCP header

2011-01-14 Thread Srini Seetharaman
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 yap...@stanford.edu 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 rob.sherw...@stanford.edu 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
 mkoba...@stanford.edu 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 Kobayashimkoba...@stanford.edu
  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 Seetharamanseeth...@stanford.edu
  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 yapyap...@stanford.edu    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 Seetharamanseeth...@stanford.edu
  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 yapyap...@stanford.edu
  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 Seetharamanseeth...@stanford.edu
  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 == 

[nox-dev] Handling corrupted TCP header

2011-01-13 Thread Srini Seetharaman
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_attcp_header(0)) {
int tcp_len = TCP_OFFSET(tcp-tcp_ctl) * 4;
if (tcp_len = sizeof *tcp) {
return reinterpret_castconst tcp_header*(b.try_pull(tcp_len));
}
}
return 0;
}
attachment: packet_with_bad_tcp_offset.PNG___
nox-dev mailing list
nox-dev@noxrepo.org
http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org


Re: [nox-dev] Handling corrupted TCP header

2011-01-13 Thread kk yap
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 seeth...@stanford.edu 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_attcp_header(0)) {
            int tcp_len = TCP_OFFSET(tcp-tcp_ctl) * 4;
            if (tcp_len = sizeof *tcp) {
                return reinterpret_castconst 
 tcp_header*(b.try_pull(tcp_len));
            }
        }
        return 0;
    }

 ___
 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

2011-01-13 Thread Srini Seetharaman
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 yap...@stanford.edu 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 seeth...@stanford.edu 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_attcp_header(0)) {
            int tcp_len = TCP_OFFSET(tcp-tcp_ctl) * 4;
            if (tcp_len = sizeof *tcp) {
                return reinterpret_castconst 
 tcp_header*(b.try_pull(tcp_len));
            }
        }
        return 0;
    }

 ___
 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

2011-01-13 Thread kk yap
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 seeth...@stanford.edu 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 yap...@stanford.edu 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 seeth...@stanford.edu 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_attcp_header(0)) {
            int tcp_len = TCP_OFFSET(tcp-tcp_ctl) * 4;
            if (tcp_len = sizeof *tcp) {
                return reinterpret_castconst 
 tcp_header*(b.try_pull(tcp_len));
            }
        }
        return 0;
    }

 ___
 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

2011-01-13 Thread Masayoshi Kobayashi

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 Kobayashimkoba...@stanford.edu  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 Seetharamanseeth...@stanford.eduwrote:


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 yapyap...@stanford.eduwrote:


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 Seetharamanseeth...@stanford.edu
  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 yapyap...@stanford.eduwrote:


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 Seetharamanseeth...@stanford.edu
  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(Bufferb)
{
if (const tcp_header *tcp = b.try_attcp_header(0)) {
int tcp_len = TCP_OFFSET(tcp-tcp_ctl) * 4;
if (tcp_len= sizeof *tcp) {
return reinterpret_castconst
tcp_header*(b.try_pull(tcp_len));
}
}
return 0;
}

___
nox-dev mailing list
nox-dev@noxrepo.org
http://noxrepo.org/mailman/listinfo/nox-dev_noxrepo.org












___
nox-dev mailing list