> On Feb 19, 2016, at 4:40 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> +pinctrl_handle_arp(const struct flow *ip_flow, struct ofpbuf *userdata)
> {
> ...
> + if (ip_flow->vlan_tci) {
> + eth_push_vlan(&packet, htons(ETH_TYPE_VLAN_8021Q),
> ip_flow->vlan_tci);
> + }
Not sure if it matters, since "ip_flow" should be pretty clean, but I think we
often check for vlans with "ip_flow->vlan_tci & htons(VLAN_CFI)".
> +static void
> +process_packet_in(const struct ofp_header *msg)
> ...
> + struct dp_packet packet;
> + dp_packet_use_const(&packet, pin.packet, pin.packet_len);
> + struct flow headers;
> + flow_extract(&packet, &headers);
> +
> + const struct flow *md = &pin.flow_metadata.flow;
> + switch (ntohl(ah->opcode)) {
> + case ACTION_OPCODE_ARP:
> + pinctrl_handle_arp(&headers, &userdata);
There's an assumption in pinctrl_handle_arp() that "headers" is an IP packet,
but I don't see any verification of that here or in that function.
> )
> +pinctrl_recv(const struct ofp_header *oh, enum ofptype type)
> {
> if (type == OFPTYPE_ECHO_REQUEST) {
> queue_msg(make_echo_reply(oh));
> } else if (type == OFPTYPE_GET_CONFIG_REPLY) {
> + /* Enable asynchronous messages. */
> struct ofputil_switch_config config;
>
> ofputil_decode_get_config_reply(oh, &config);
> config.miss_send_len = UINT16_MAX;
> set_switch_config(swconn, &config);
Since it's not obvious that turning on a non-zero miss_send_len enables async
messages, it might be worth referring people to DESIGN.md.
> static void
> +parse_arp_action(struct action_context *ctx)
> +{
> ...
> + /* controller. */
It might be nice to say a little more about sending this to the controller,
since it doesn't fit the other comment styles in this section.
--Justin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
