diff --git a/datapath/datapath.c b/datapath/datapath.c index afa851d..b2d7bd5 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -659,7 +659,9 @@ int dp_output_port(struct datapath *dp, struct sk_buff *skb, int out_port, return output_all(dp, skb, 0); case OFPP_CONTROLLER: - return dp_output_control(dp, skb, 0, OFPR_ACTION); + return dp_output_control(dp, skb, + PKTSIZ_ENOUGH_TO_CARRY_ENTIRE_PACKET, + OFPR_ACTION); case OFPP_LOCAL: { struct net_device *dev = dp->netdev; @@ -750,8 +752,8 @@ out: /* Takes ownership of 'skb' and transmits it to 'dp''s control path. 'reason' * indicates why 'skb' is being sent. 'max_len' sets the maximum number of - * bytes that the caller wants to be sent; a value of 0 indicates the entire - * packet should be sent. */ + * bytes that the caller wants to be sent. + */ int dp_output_control(struct datapath *dp, struct sk_buff *skb, size_t max_len, int reason) @@ -783,7 +785,7 @@ dp_output_control(struct datapath *dp, struct sk_buff *skb, buffer_id = fwd_save_skb(skb); fwd_len = skb->len; - if ((buffer_id != (uint32_t) -1) && max_len) + if (buffer_id != (uint32_t) -1) fwd_len = min(fwd_len, max_len); opi_len = offsetof(struct ofp_packet_in, data) + fwd_len; diff --git a/datapath/dp_act.c b/datapath/dp_act.c index 4a8f71b..3bdd47c 100644 --- a/datapath/dp_act.c +++ b/datapath/dp_act.c @@ -480,7 +480,7 @@ void execute_actions(struct datapath *dp, struct sk_buff *skb, * then freeing the original skbuff is wasteful. So the following code * is slightly obscure just to avoid that. */ int prev_port; - size_t max_len=0; /* Initialze to make compiler happy */ + size_t max_len = PKTSIZ_ENOUGH_TO_CARRY_ENTIRE_PACKET; uint8_t *p = (uint8_t *)actions; prev_port = -1; diff --git a/datapath/forward.h b/datapath/forward.h index f69a8f0..ba0ce48 100644 --- a/datapath/forward.h +++ b/datapath/forward.h @@ -20,6 +20,7 @@ struct sender; #define PKT_COOKIE_BITS (32 - PKT_BUFFER_BITS) +#define PKTSIZ_ENOUGH_TO_CARRY_ENTIRE_PACKET 65535 void fwd_port_input(struct sw_chain *, struct sk_buff *, struct net_bridge_port *); diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h index 8458d69..733a954 100644 --- a/include/openflow/openflow.h +++ b/include/openflow/openflow.h @@ -341,8 +341,7 @@ enum ofp_action_type { /* Action structure for OFPAT_OUTPUT, which sends packets out 'port'. * When the 'port' is the OFPP_CONTROLLER, 'max_len' indicates the max - * number of bytes to send. A 'max_len' of zero means the entire packet - * should be sent. */ + * number of bytes to send. */ struct ofp_action_output { uint16_t type; /* OFPAT_OUTPUT. */ uint16_t len; /* Length is 8. */ diff --git a/lib/vconn.c b/lib/vconn.c index 687f795..67562a0 100644 --- a/lib/vconn.c +++ b/lib/vconn.c @@ -923,6 +923,8 @@ make_add_simple_flow(const struct flow *flow, oao->type = htons(OFPAT_OUTPUT); oao->len = htons(sizeof *oao); oao->port = htons(out_port); + if (oao->port == htons(OFPP_CONTROLLER)) + oao->max_len = htons(65535); /* Enough to carry entire packet */ return buffer; } diff --git a/regress/lib/Perl5/OF/OFUtil.pm b/regress/lib/Perl5/OF/OFUtil.pm index 2f254d6..419d1f2 100644 --- a/regress/lib/Perl5/OF/OFUtil.pm +++ b/regress/lib/Perl5/OF/OFUtil.pm @@ -871,12 +871,19 @@ sub combine_args { #and No action for drops my $action_output_args; my $action_output; + + my $max_len; + if ($out_port != $enums{'OFPP_CONTROLLER'}) { + $max_len = 0; + } else { + $max_len = 65535; + } if ($mod_type ne 'drop') { $action_output_args = { type => $enums{'OFPAT_OUTPUT'}, len => $ofp->sizeof('ofp_action_output'), port => $out_port, - max_len => 0, # send entire packet + max_len => $max_len, }; $action_output = $ofp->pack('ofp_action_output', $action_output_args); } @@ -1737,11 +1744,17 @@ sub create_flow_mod_from_icmp_action { tp_dst => $icmp_code }; + my $max_len; + if ($out_port != $enums{'OFPP_CONTROLLER'}) { + $max_len = 0; + } else { + $max_len = 65535; + } my $action_output_args = { type => $enums{'OFPAT_OUTPUT'}, len => $ofp->sizeof('ofp_action_output'), port => $out_port, - max_len => 0, # send entire packet + max_len => $max_len, }; my $action_output = $ofp->pack( 'ofp_action_output', $action_output_args ); diff --git a/regress/projects/black_box/regress/test_miss_send_length/run.pl b/regress/projects/black_box/regress/test_miss_send_length/run.pl index a40545c..454fe9e 100755 --- a/regress/projects/black_box/regress/test_miss_send_length/run.pl +++ b/regress/projects/black_box/regress/test_miss_send_length/run.pl @@ -5,45 +5,71 @@ use strict; use OF::Includes; +sub verify_packet_in { + my ($sock, $options_ref, $miss_send_len, $pktsiz, $expected_pktsiz) = @_; + + my $in_port = $$options_ref{'port_base'}; + my $out_port = $in_port + 1; + + my $pkt = get_default_black_box_pkt_len($in_port, $out_port, $pktsiz); + nftest_send('eth1', $pkt->packed); + + my $rcvd_msg; + sysread($sock, $rcvd_msg, 1512) || die "Failed to receive message: $!"; + + # Inspect message + my $msg_size = length($rcvd_msg); + compare("msg size", $msg_size, '==', $expected_pktsiz); + + my $msg = $ofp->unpack('ofp_packet_in', $rcvd_msg); + #print HexDump ($rcvd_msg); + #print Dumper($msg); + + # Verify fields + verify_header($msg, 'OFPT_PACKET_IN', $msg_size); + + # total len should be full length of original sent frame + compare("total len", $$msg{'total_len'}, '==', length($pkt->packed)); + compare("in_port", $$msg{'in_port'}, '==', $in_port); + compare("reason", $$msg{'reason'}, '==', $enums{'OFPR_NO_MATCH'}); + + # verify packet was unchanged! + my $rcvd_pkt_data = substr($rcvd_msg, $ofp->offsetof('ofp_packet_in', 'data')); + + # trim to MISS_SEND_LEN + my $pkt_trimmed = substr($pkt->packed, 0, $miss_send_len); + if ($rcvd_pkt_data ne $pkt_trimmed) { + die "ERROR: received packet data didn't match packet sent\n"; + } +} + sub my_test { - - my ($sock, $options_ref) = @_; + my ($sock, $options_ref) = @_; - my $in_port = $$options_ref{'port_base'}; - my $out_port = $in_port + 1; + my $miss_send_len = get_of_miss_send_len_default(); + my $pktsiz = 63; + my $expected_pktsiz = 8 + 10 + $pktsiz; + verify_packet_in($sock, $options_ref, $miss_send_len, $pktsiz, $expected_pktsiz); - my $pkt = get_default_black_box_pkt_len( $in_port, $out_port, 256); + $miss_send_len = 0; + $pktsiz = 67; + my $expected_pktsiz = 8 + 10; + set_config($ofp, $sock, $options_ref, 1, $miss_send_len); + verify_packet_in($sock, $options_ref, $miss_send_len, $pktsiz, $expected_pktsiz); - nftest_send( 'eth1', $pkt->packed); + $miss_send_len = 127; + $pktsiz = 259; + my $expected_pktsiz = 8 + 10 + $miss_send_len; + set_config($ofp, $sock, $options_ref, 1, $miss_send_len); + verify_packet_in($sock, $options_ref, $miss_send_len, $pktsiz, $expected_pktsiz); - my $recvd_mesg; - sysread($sock, $recvd_mesg, 1512) || die "Failed to receive message: $!"; + $miss_send_len = 65535; + $pktsiz = 1500 - 8 - 10; + my $expected_pktsiz = 1500; + set_config($ofp, $sock, $options_ref, 1, $miss_send_len); + verify_packet_in($sock, $options_ref, $miss_send_len, $pktsiz, $expected_pktsiz); - # Inspect message - my $msg_size = length($recvd_mesg); - my $expected_size = $ofp->offsetof('ofp_packet_in', 'data') + get_of_miss_send_len_default(); - compare ("msg size", $msg_size, '==', $expected_size); - - my $msg = $ofp->unpack('ofp_packet_in', $recvd_mesg); - #print HexDump ($recvd_mesg); - #print Dumper($msg); - - # Verify fields - verify_header($msg, 'OFPT_PACKET_IN', $msg_size); - - # total len should be full length of original sent frame - compare("total len", $$msg{'total_len'}, '==', length($pkt->packed)); - compare("in_port", $$msg{'in_port'}, '==', $in_port); - compare("reason", $$msg{'reason'}, '==', $enums{'OFPR_NO_MATCH'}); - - # verify packet was unchanged! - my $recvd_pkt_data = substr ($recvd_mesg, $ofp->offsetof('ofp_packet_in', 'data')); - - # trim to MISS_SEND_LEN - my $pkt_trimmed = substr ($pkt->packed, 0, get_of_miss_send_len_default()); - if ($recvd_pkt_data ne $pkt_trimmed) { - die "ERROR: received packet data didn't match packet sent\n"; - } + set_config($ofp, $sock, $options_ref, 1, get_of_miss_send_len_default()); } run_black_box_test(\&my_test, \@ARGV); diff --git a/udatapath/datapath.c b/udatapath/datapath.c index 9b30128..49c120d 100644 --- a/udatapath/datapath.c +++ b/udatapath/datapath.c @@ -563,7 +563,8 @@ dp_output_port(struct datapath *dp, struct ofpbuf *buffer, break; case OFPP_CONTROLLER: - dp_output_control(dp, buffer, in_port, 0, OFPR_ACTION); + dp_output_control(dp, buffer, in_port, + PKTSIZ_ENOUGH_TO_CARRY_ENTIRE_PACKET, OFPR_ACTION); break; case OFPP_LOCAL: @@ -627,8 +628,7 @@ send_openflow_buffer(struct datapath *dp, struct ofpbuf *buffer, * packet can be saved in a buffer, then only the first max_len bytes of * 'buffer' are sent; otherwise, all of 'buffer' is sent. 'reason' indicates * why 'buffer' is being sent. 'max_len' sets the maximum number of bytes that - * the caller wants to be sent; a value of 0 indicates the entire packet should - * be sent. */ + * the caller wants to be sent. */ void dp_output_control(struct datapath *dp, struct ofpbuf *buffer, int in_port, size_t max_len, int reason) @@ -639,7 +639,7 @@ dp_output_control(struct datapath *dp, struct ofpbuf *buffer, int in_port, buffer_id = save_buffer(buffer); total_len = buffer->size; - if (buffer_id != UINT32_MAX && max_len && buffer->size > max_len) { + if (buffer_id != UINT32_MAX && buffer->size > max_len) { buffer->size = max_len; } diff --git a/udatapath/datapath.h b/udatapath/datapath.h index 37cbdbe..8208c55 100644 --- a/udatapath/datapath.h +++ b/udatapath/datapath.h @@ -63,6 +63,8 @@ struct sw_port { #define DP_MAX_PORTS 255 BUILD_ASSERT_DECL(DP_MAX_PORTS <= OFPP_MAX); +#define PKTSIZ_ENOUGH_TO_CARRY_ENTIRE_PACKET 65535 + struct datapath { /* Remote connections. */ struct list remotes; /* All connections (including controller). */ diff --git a/udatapath/dp_act.c b/udatapath/dp_act.c index 26edcc0..f430f0c 100644 --- a/udatapath/dp_act.c +++ b/udatapath/dp_act.c @@ -470,7 +470,7 @@ void execute_actions(struct datapath *dp, struct ofpbuf *buffer, * freeing the original buffer is wasteful. So the following code is * slightly obscure just to avoid that. */ int prev_port; - size_t max_len=0; /* Initialze to make compiler happy */ + size_t max_len = PKTSIZ_ENOUGH_TO_CARRY_ENTIRE_PACKET; uint16_t in_port = ntohs(key->flow.in_port); uint8_t *p = (uint8_t *)actions; @@ -483,8 +483,8 @@ void execute_actions(struct datapath *dp, struct ofpbuf *buffer, size_t len = htons(ah->len); if (prev_port != -1) { - do_output(dp, ofpbuf_clone(buffer), in_port, max_len, - prev_port, ignore_no_fwd); + do_output(dp, ofpbuf_clone(buffer), in_port, max_len, + prev_port, ignore_no_fwd); prev_port = -1; } diff --git a/utilities/wireshark_dissectors/openflow/packet-openflow.c b/utilities/wireshark_dissectors/openflow/packet-openflow.c index 8e593c4..8a52978 100644 --- a/utilities/wireshark_dissectors/openflow/packet-openflow.c +++ b/utilities/wireshark_dissectors/openflow/packet-openflow.c @@ -2175,14 +2175,10 @@ static void dissect_action_output(proto_tree* tree, tvbuff_t *tvb, guint32 *offs dissect_port( tree, ofp_action_output_port, tvb, offset ); /* determine the maximum number of bytes to send (0 => no limit) */ - guint16 max_len = tvb_get_ntohs( tvb, *offset ); - if( max_len ) { - char str[11]; - snprintf( str, 11, "%u", max_len ); - add_child_str( tree, ofp_action_output_max_len, tvb, offset, 2, str ); - } - else - add_child_str( tree, ofp_action_output_max_len, tvb, offset, 2, "entire packet (no limit)" ); + guint16 max_len = tvb_get_ntohs( tvb, *offset ); + char str[11]; + snprintf( str, 11, "%u", max_len ); + add_child_str( tree, ofp_action_output_max_len, tvb, offset, 2, str ); } /** returns the number of bytes dissected (-1 if an unknown action type is @@ -2614,18 +2610,18 @@ static void dissect_openflow_message(tvbuff_t *tvb, packet_info *pinfo, proto_tr add_child(type_tree, ofp_packet_in_reason, tvb, &offset, 1); dissect_pad(type_tree, &offset, 1); - /* continue the dissection with the Ethernet dissector */ - if( data_ethernet ) { - proto_item *data_item = proto_tree_add_item(type_tree, ofp_packet_in_data_hdr, tvb, offset, -1, FALSE); - proto_tree *data_tree = proto_item_add_subtree(data_item, ett_ofp_packet_in_data_hdr); - tvbuff_t *next_tvb = tvb_new_subset(tvb, offset, -1, total_len); - dissect_ethernet(next_tvb, pinfo, data_tree); + if (len > sizeof(struct ofp_packet_in)) { + /* continue the dissection with the Ethernet dissector */ + if (data_ethernet) { + proto_item *data_item = proto_tree_add_item(type_tree, ofp_packet_in_data_hdr, tvb, offset, -1, FALSE); + proto_tree *data_tree = proto_item_add_subtree(data_item, ett_ofp_packet_in_data_hdr); + tvbuff_t *next_tvb = tvb_new_subset(tvb, offset, -1, total_len); + dissect_ethernet(next_tvb, pinfo, data_tree); + } else { + /* if we couldn't load the ethernet dissector, just display the bytes */ + add_child(type_tree, ofp_packet_in_data_hdr, tvb, &offset, total_len); + } } - else { - /* if we couldn't load the ethernet dissector, just display the bytes */ - add_child(type_tree, ofp_packet_in_data_hdr, tvb, &offset, total_len); - } - break; }