Netdev List
 help / color / mirror / Atom feed
* [PATCH v2.45 1/4] odp: Allow VLAN actions after MPLS actions
From: Simon Horman @ 2013-10-25 14:34 UTC (permalink / raw)
  To: dev, netdev, Jesse Gross, Ben Pfaff
  Cc: Pravin B Shelar, Ravi K, Isaku Yamahata, Joe Stringer
In-Reply-To: <1382711684-17080-1-git-send-email-horms@verge.net.au>

From: Joe Stringer <joe@wand.net.nz>

OpenFlow 1.1 and 1.2, and 1.3 differ in their handling of MPLS actions in the
presence of VLAN tags. To allow correct behaviour to be committed in
each situation, this patch adds a second round of VLAN tag action
handling to commit_odp_actions(), which occurs after MPLS actions. This
is implemented with a new field in 'struct xlate_in' called 'vlan_tci'.

When an push_mpls action is composed, the flow's current VLAN state is
stored into xin->vlan_tci, and flow->vlan_tci is set to 0 (pop_vlan). If
a VLAN tag is present, it is stripped; if not, then there is no change.
Any later modifications to the VLAN state is written to xin->vlan_tci.
When committing the actions, flow->vlan_tci is used before MPLS actions,
and xin->vlan_tci is used afterwards. This retains the current datapath
behaviour, but allows VLAN actions to be applied in a more flexible
manner.

Both before and after this patch MPLS LSEs are pushed onto a packet after
any VLAN tags that may be present. This is the behaviour described in
OpenFlow 1.1 and 1.2. OpenFlow 1.3 specifies that MPLS LSEs should be
pushed onto a packet before any VLAN tags that are present. Support
for this will be added by a subsequent patch that makes use of
the infrastructure added by this patch.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v2.45 [Simon Horman]
* As pointed out by Ben Pfaff and Joe Stringer
  + Update VLAN handling in the presence of MPLS push

    Previously the test for committing ODP VLAN actions after MPLS actions
    was that the VLAN TCI differed before and after the MPLS push action.
    This results in a false negative in some cases including if a VLAN tag
    is pushed after the MPLS push action in such a way that it duplicates
    the VLAN tag present before the MPLS push action.

    This is resolved by ensuring the VLAN_CFI bit of the value used to
    track the desired VLAN TCI after an MPLS push action is zero unless
    VLAN actions should be committed after MPLS actions.

  + Update tests to use ovs-ofctl monitor "-m" to allow full inspection of
    MPLS LSE and VLAN tags present in packets.

v2.44 [Simon Horman]
* Rebase for the following changes:
  f47ea02 ("Set datapath mask bits when setting a flow field.")
  7fdb60a ("Add support for write-actions")
  7358063 ("odp-util: Elaborate the comment for odp_flow_format() function.")
* Correct final_vlan_tci and next_vlan_tci initialisation in xlate_actions__()

v2.43 [Simon Horman]
* As suggested by Ben Pfaff
  Move vlan state into struct xlate_ctx

  1. Add final_vlan_tci member to struct xlate_ctx instead of vlan_tci member
     struct xlate_xin.  This seems to be a better palace for it as it does
     not need to be accessible from the caller.

  2. Move local vlan_tci variable of do_xlate_actions() to be the
     next_vlan_tci member of strict xlate_ctx.  This allows for it to work
     across resubmit actions and goto table instructions.

v2.42
* No change

v2.41 [Joe Stringer via Simon Horman]
* Rework comments to make things a little clearer

v2.40 [Simon Horman]
* Rebase for removal of mpls_depth from struct flow

v2.38 - v2.39
* No change

v2.37
* Rebase

v2.36
* No change

v2.5 [Joe Stringer]
* First post
---
 lib/odp-util.c               |  12 +-
 lib/odp-util.h               |   3 +-
 ofproto/ofproto-dpif-xlate.c | 136 ++++++++++++---
 tests/ofproto-dpif.at        | 389 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 514 insertions(+), 26 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 6875e01..21b33ac 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3639,11 +3639,15 @@ commit_set_pkt_mark_action(const struct flow *flow, struct flow *base,
  * used as part of the action.
  *
  * Returns a reason to force processing the flow's packets into the userspace
- * slow path, if there is one, otherwise 0. */
+ * slow path, if there is one, otherwise 0.
+ *
+ * VLAN actions may be committed twice; If vlan_tci in 'flow' differs from the
+ * one in 'base', then it is committed before MPLS actions. If the VLAN_CFI
+ * bit of 'post_mpls_vlan_tci' is set then it is committed afterwards. */
 enum slow_path_reason
 commit_odp_actions(const struct flow *flow, struct flow *base,
                    struct ofpbuf *odp_actions, struct flow_wildcards *wc,
-                   int *mpls_depth_delta)
+                   int *mpls_depth_delta, ovs_be16 post_mpls_vlan_tci)
 {
     enum slow_path_reason slow;
 
@@ -3656,6 +3660,10 @@ commit_odp_actions(const struct flow *flow, struct flow *base,
      * that it is no longer IP and thus nw and port actions are no longer valid.
      */
     commit_mpls_action(flow, base, odp_actions, wc, mpls_depth_delta);
+    if (post_mpls_vlan_tci & htons(VLAN_CFI)) {
+        base->vlan_tci = htons(0);
+        commit_vlan_action(post_mpls_vlan_tci, base, odp_actions, wc);
+    }
     commit_set_priority_action(flow, base, odp_actions, wc);
     commit_set_pkt_mark_action(flow, base, odp_actions, wc);
 
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 821b2c4..636a3ec 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -175,7 +175,8 @@ enum slow_path_reason commit_odp_actions(const struct flow *,
                                          struct flow *base,
                                          struct ofpbuf *odp_actions,
                                          struct flow_wildcards *wc,
-                                         int *mpls_depth_delta);
+                                         int *mpls_depth_delta,
+                                         ovs_be16 post_mpls_vlan_tci);
 \f
 /* ofproto-dpif interface.
  *
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7be691c..d79356f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -180,6 +180,37 @@ struct xlate_ctx {
     uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
     bool exit;                  /* No further actions should be processed. */
 
+    /* The post MPLS vlan_tci.
+     *
+     * The value of the vlan TCI prior to the translation of an MPLS push
+     * actions should be stored in 'xin->flow->vlan_tci'.
+     *
+     * If an VLAN or set field action is subsequently translated then
+     * 'post_mpls_vlan_tci' is updated as according to the action.
+     *
+     * If the VLAN_CFI bit of 'post_mpls_vlan_tci' is set then VLAN ODP actions
+     * will be committed after any MPLS actions regardless of whether VLAN
+     * actions were also committed before the MPLS actions or not.
+     *
+     * This mechanism allows a VLAN tag to be popped before pushing
+     * an MPLS LSE and then the same VLAN tag pushed after pushing
+     * the MPLS LSE. In this way it is possible to push an MPLS LSE
+     * after an existing VLAN tag. Moreover this mechanism allows
+     * the order in which VLAN tags and MPLS LSEs are pushed. */
+    ovs_be16 post_mpls_vlan_tci;
+
+    /* The next vlan_tci state.
+     *
+     * This field pints to the variable update each time an
+     * action updates the VLAN tci.
+     *
+     * This variable initially points to 'xin->flow->vlan_tci' so that ODP
+     * VLAN actions are committed before any MPLS actions. When an MPLS
+     * action is composed 'next_vlan_tci' is updated to point to
+     * 'post_mpls_vlan_tci'. This causes subsequent VLAN actions to be
+     * committed after MPLS actions. */
+    ovs_be16 *next_vlan_tci;
+
     /* OpenFlow 1.1+ action set.
      *
      * 'action_set' accumulates "struct ofpact"s added by OFPACT_WRITE_ACTIONS.
@@ -996,11 +1027,38 @@ output_vlan_to_vid(const struct xbundle *out_xbundle, uint16_t vlan)
     }
 }
 
+static bool mpls_actions_xlated(struct xlate_ctx *ctx)
+{
+    return ctx->next_vlan_tci == &ctx->post_mpls_vlan_tci;
+}
+
+static ovs_be16
+vlan_tci_save(struct xlate_ctx *ctx)
+{
+    ovs_be16 orig_tci = ctx->xin->flow.vlan_tci;
+    /* If MPLS actions were executed after vlan actions then
+     * copy the final vlan_tci out and restore the intermediate VLAN state. */
+    if (mpls_actions_xlated(ctx)) {
+        ctx->xin->flow.vlan_tci = *ctx->next_vlan_tci;
+    }
+    return orig_tci;
+}
+
+static void
+vlan_tci_restore(struct xlate_ctx *ctx, ovs_be16 orig_tci)
+{
+    /* If MPLS actions were executed after vlan actions then
+     * copy the final vlan_tci out and restore the intermediate VLAN state. */
+    if (mpls_actions_xlated(ctx)) {
+        ctx->post_mpls_vlan_tci = ctx->xin->flow.vlan_tci;
+        ctx->xin->flow.vlan_tci = orig_tci;
+    }
+}
+
 static void
 output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
               uint16_t vlan)
 {
-    ovs_be16 *flow_tci = &ctx->xin->flow.vlan_tci;
     uint16_t vid;
     ovs_be16 tci, old_tci;
     struct xport *xport;
@@ -1025,18 +1083,18 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
         }
     }
 
-    old_tci = *flow_tci;
+    old_tci = *ctx->next_vlan_tci;
     tci = htons(vid);
     if (tci || out_xbundle->use_priority_tags) {
-        tci |= *flow_tci & htons(VLAN_PCP_MASK);
+        tci |= *ctx->next_vlan_tci & htons(VLAN_PCP_MASK);
         if (tci) {
             tci |= htons(VLAN_CFI);
         }
     }
-    *flow_tci = tci;
+    ctx->xin->flow.vlan_tci = *ctx->next_vlan_tci = tci;
 
     compose_output_action(ctx, xport->ofp_port);
-    *flow_tci = old_tci;
+    ctx->xin->flow.vlan_tci = *ctx->next_vlan_tci = old_tci;
 }
 
 /* A VM broadcasts a gratuitous ARP to indicate that it has resumed after
@@ -1269,7 +1327,7 @@ xlate_normal(struct xlate_ctx *ctx)
 
     /* Drop malformed frames. */
     if (flow->dl_type == htons(ETH_TYPE_VLAN) &&
-        !(flow->vlan_tci & htons(VLAN_CFI))) {
+        !(*ctx->next_vlan_tci & htons(VLAN_CFI))) {
         if (ctx->xin->packet != NULL) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
             VLOG_WARN_RL(&rl, "bridge %s: dropping packet with partial "
@@ -1293,7 +1351,7 @@ xlate_normal(struct xlate_ctx *ctx)
     }
 
     /* Check VLAN. */
-    vid = vlan_tci_to_vid(flow->vlan_tci);
+    vid = vlan_tci_to_vid(*ctx->next_vlan_tci);
     if (!input_vid_is_valid(vid, in_xbundle, ctx->xin->packet != NULL)) {
         xlate_report(ctx, "disallowed VLAN VID for this input port, dropping");
         return;
@@ -1551,7 +1609,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
     struct flow_wildcards *wc = &ctx->xout->wc;
     struct flow *flow = &ctx->xin->flow;
-    ovs_be16 flow_vlan_tci;
+    ovs_be16 flow_vlan_tci, vlan_tci;
     uint32_t flow_pkt_mark;
     uint8_t flow_nw_tos;
     odp_port_t out_port, odp_port;
@@ -1620,6 +1678,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     }
 
     flow_vlan_tci = flow->vlan_tci;
+    vlan_tci = *ctx->next_vlan_tci;
     flow_pkt_mark = flow->pkt_mark;
     flow_nw_tos = flow->nw_tos;
 
@@ -1659,12 +1718,13 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
             wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
         }
         vlandev_port = vsp_realdev_to_vlandev(ctx->xbridge->ofproto, ofp_port,
-                                              flow->vlan_tci);
+                                              *ctx->next_vlan_tci);
         if (vlandev_port == ofp_port) {
             out_port = odp_port;
         } else {
             out_port = ofp_port_to_odp_port(ctx->xbridge, vlandev_port);
             flow->vlan_tci = htons(0);
+            ctx->post_mpls_vlan_tci = htons(0);
         }
     }
 
@@ -1672,7 +1732,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
                                               &ctx->xout->odp_actions,
                                               &ctx->xout->wc,
-                                              &ctx->mpls_depth_delta);
+                                              &ctx->mpls_depth_delta,
+                                              ctx->post_mpls_vlan_tci);
         nl_msg_put_odp_port(&ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
                             out_port);
 
@@ -1684,6 +1745,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
  out:
     /* Restore flow */
     flow->vlan_tci = flow_vlan_tci;
+    *ctx->next_vlan_tci = vlan_tci;
     flow->pkt_mark = flow_pkt_mark;
     flow->nw_tos = flow_nw_tos;
 }
@@ -1838,7 +1900,8 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                           &ctx->xout->odp_actions,
                                           &ctx->xout->wc,
-                                          &ctx->mpls_depth_delta);
+                                          &ctx->mpls_depth_delta,
+                                          ctx->post_mpls_vlan_tci);
 
     odp_execute_actions(NULL, packet, &key, ctx->xout->odp_actions.data,
                         ctx->xout->odp_actions.size, NULL, NULL);
@@ -2231,7 +2294,8 @@ xlate_sample_action(struct xlate_ctx *ctx,
   ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                         &ctx->xout->odp_actions,
                                         &ctx->xout->wc,
-                                        &ctx->mpls_depth_delta);
+                                        &ctx->mpls_depth_delta,
+                                        ctx->post_mpls_vlan_tci);
 
   compose_flow_sample_cookie(os->probability, os->collector_set_id,
                              os->obs_domain_id, os->obs_point_id, &cookie);
@@ -2320,28 +2384,28 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
 
         case OFPACT_SET_VLAN_VID:
             wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
-            flow->vlan_tci &= ~htons(VLAN_VID_MASK);
-            flow->vlan_tci |= (htons(ofpact_get_SET_VLAN_VID(a)->vlan_vid)
-                               | htons(VLAN_CFI));
+            *ctx->next_vlan_tci &= ~htons(VLAN_VID_MASK);
+            *ctx->next_vlan_tci |= (htons(ofpact_get_SET_VLAN_VID(a)->vlan_vid)
+                          | htons(VLAN_CFI));
             break;
 
         case OFPACT_SET_VLAN_PCP:
             wc->masks.vlan_tci |= htons(VLAN_PCP_MASK | VLAN_CFI);
-            flow->vlan_tci &= ~htons(VLAN_PCP_MASK);
-            flow->vlan_tci |=
+            *ctx->next_vlan_tci &= ~htons(VLAN_PCP_MASK);
+            *ctx->next_vlan_tci |=
                 htons((ofpact_get_SET_VLAN_PCP(a)->vlan_pcp << VLAN_PCP_SHIFT)
                       | VLAN_CFI);
             break;
 
         case OFPACT_STRIP_VLAN:
             memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
-            flow->vlan_tci = htons(0);
+            *ctx->next_vlan_tci = htons(0);
             break;
 
         case OFPACT_PUSH_VLAN:
             /* XXX 802.1AD(QinQ) */
             memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
-            flow->vlan_tci = htons(VLAN_CFI);
+            *ctx->next_vlan_tci = htons(VLAN_CFI);
             break;
 
         case OFPACT_SET_ETH_SRC:
@@ -2423,29 +2487,53 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             flow->skb_priority = ctx->orig_skb_priority;
             break;
 
-        case OFPACT_REG_MOVE:
+        case OFPACT_REG_MOVE: {
+            ovs_be16 orig_tci = flow->vlan_tci;
             nxm_execute_reg_move(ofpact_get_REG_MOVE(a), flow, wc);
+            vlan_tci_restore(ctx, orig_tci);
             break;
+        }
 
-        case OFPACT_REG_LOAD:
+        case OFPACT_REG_LOAD: {
+            ovs_be16 orig_tci = vlan_tci_save(ctx);
             nxm_execute_reg_load(ofpact_get_REG_LOAD(a), flow, wc);
+            vlan_tci_restore(ctx, orig_tci);
             break;
+        }
 
-        case OFPACT_STACK_PUSH:
+        case OFPACT_STACK_PUSH: {
+            ovs_be16 orig_tci = vlan_tci_save(ctx);
             nxm_execute_stack_push(ofpact_get_STACK_PUSH(a), flow, wc,
                                    &ctx->stack);
+            vlan_tci_restore(ctx, orig_tci);
             break;
+        }
 
-        case OFPACT_STACK_POP:
+        case OFPACT_STACK_POP: {
+            ovs_be16 orig_tci = vlan_tci_save(ctx);
             nxm_execute_stack_pop(ofpact_get_STACK_POP(a), flow, wc,
                                   &ctx->stack);
+            vlan_tci_restore(ctx, orig_tci);
             break;
+        }
 
         case OFPACT_PUSH_MPLS:
             if (compose_mpls_push_action(ctx,
                                          ofpact_get_PUSH_MPLS(a)->ethertype)) {
                 return;
             }
+
+            /* Save and pop any existing VLAN tags. They will be pushed
+             * back onto the packet after pushing the MPLS LSE. The overall
+             * effect is to push the MPLS LSE after any VLAN tags that may
+             * be present. This is the behaviour described for OpenFlow 1.1
+             * and 1.2. This code needs to be enhanced to make this
+             * conditional to also and support pushing the MPLS LSE before
+             * any VLAN tags that may be present, the behaviour described
+             * for OpenFlow 1.3. */
+            ctx->post_mpls_vlan_tci = *ctx->next_vlan_tci;
+            flow->vlan_tci = htons(0);
+            ctx->next_vlan_tci = &ctx->post_mpls_vlan_tci;
             break;
 
         case OFPACT_POP_MPLS:
@@ -2786,6 +2874,8 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
     ctx.table_id = 0;
     ctx.exit = false;
     ctx.mpls_depth_delta = 0;
+    ctx.post_mpls_vlan_tci = htons(0);
+    ctx.next_vlan_tci = &ctx.xin->flow.vlan_tci;
 
     if (!xin->ofpacts && !ctx.rule) {
         rule_dpif_lookup(ctx.xbridge->ofproto, flow, wc, &rule);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index c569463..372bce7 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -965,6 +965,395 @@ done
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - OF1.2 VLAN+MPLS handling])
+OVS_VSWITCHD_START([dnl
+   add-port br0 p1 -- set Interface p1 type=dummy
+])
+ON_EXIT([kill `cat ovs-ofctl.pid`])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_DATA([flows.txt], [dnl
+cookie=0xa dl_src=40:44:44:44:54:50 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,controller
+cookie=0xa dl_src=40:44:44:44:54:51 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,controller
+cookie=0xa dl_src=40:44:44:44:54:52 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,controller
+cookie=0xa dl_src=40:44:44:44:54:53 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,controller
+cookie=0xa dl_src=40:44:44:44:54:54 actions=push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
+cookie=0xa dl_src=40:44:44:44:54:55 actions=push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
+cookie=0xa dl_src=40:44:44:44:54:56 actions=push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
+cookie=0xa dl_src=40:44:44:44:54:57 actions=push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
+cookie=0xa dl_src=40:44:44:44:54:58 actions=load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
+cookie=0xa dl_src=40:44:44:44:54:59 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],mod_vlan_pcp:1,load:99->OXM_OF_VLAN_VID[[]],controller
+])
+AT_CHECK([ovs-ofctl --protocols=OpenFlow12 add-flows br0 flows.txt])
+
+dnl Modified MPLS controller action.
+dnl In this test, we push the MPLS tag before pushing a VLAN tag, so we see
+dnl both of these in the final flow
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:50,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
+done
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 50 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 50 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 50 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl before we push the MPLS and VLAN tags.
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:51,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:51,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 51 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:51,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 51 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:51,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 51 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, we push the MPLS tag before pushing a VLAN tag, so we see
+dnl both of these in the final flow
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:52,dst=52:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
+done
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  52 54 00 00 00 07 40 44-44 44 54 52 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  52 54 00 00 00 07 40 44-44 44 54 52 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  52 54 00 00 00 07 40 44-44 44 54 52 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl before we push the MPLS and VLAN tags.
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:53,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:53,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 53 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:53,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 53 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:53,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 53 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, we push the VLAN tag before pushing a MPLS tag, but these
+dnl actions are reordered, so we see both of these in the final flow.
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:54,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 54 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 54 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 54 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl before we push the MPLS and VLAN tags.
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:55,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:55,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 55 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:55,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 55 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:55,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 55 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, we push the VLAN tag before pushing a MPLS tag, but these
+dnl actions are reordered, so we see both of these in the final flow.
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:56,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 56 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 56 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 56 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+00000040  00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl before we push the MPLS and VLAN tags.
+AT_CHECK([ovs-ofctl monitor br0 -m 65534 -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:57,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 57 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 57 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 57 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl before we push the MPLS and VLAN tags.
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:58,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 58 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 58 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 58 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+])
+
+dnl Modified MPLS controller action.
+dnl In this test, the input packet in vlan-tagged, which should be modified
+dnl before we push MPLS and VLAN tags.
+AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:59,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:59,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 59 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:59,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 59 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
+mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:59,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
+00000000  50 54 00 00 00 07 40 44-44 44 54 59 81 00 20 63
+00000010  88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
+00000020  f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
+00000030  00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
+])
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:50 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],mod_vlan_vid:99,mod_vlan_pcp:1,CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:51 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],mod_vlan_vid:99,mod_vlan_pcp:1,CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:52 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:53 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:54 actions=mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:55 actions=mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:56 actions=load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:57 actions=load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:58 actions=load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:59 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],mod_vlan_pcp:1,load:0x63->OXM_OF_VLAN_VID[[]],CONTROLLER:65535
+NXST_FLOW reply:
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - fragment handling])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [1], [2], [3], [4], [5], [6], [90])
-- 
1.8.4

^ permalink raw reply related

* [PATCH net 0/2] qlcnic: Bug fixes
From: Shahed Shaikh @ 2013-10-25 14:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Shahed Shaikh

From: Shahed Shaikh <shahed.shaikh@qlogic.com>

This patch series contains following fixes-
* Performace drop because driver was forcing adapter not to check
  destination IP for LRO.
* driver was not issuing qlcnic_fw_cmd_set_drv_version() to 83xx adapter
  becasue of improper handling of QLCNIC_FW_CAPABILITY_MORE_CAPS bit.

Please apply to net.

Thanks,
Shahed

Shahed Shaikh (2):
  qlcnic: Do not force adapter to perform LRO without destination IP
    check
  qlcnic: Do not read QLCNIC_FW_CAPABILITY_MORE_CAPS bit for 83xx
    adapter

 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 6 +++---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c      | 7 ++-----
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c    | 6 ++++--
 3 files changed, 9 insertions(+), 10 deletions(-)

-- 
1.8.1.4

^ permalink raw reply

* [PATCH net 2/2] qlcnic: Do not read QLCNIC_FW_CAPABILITY_MORE_CAPS bit for 83xx adapter
From: Shahed Shaikh @ 2013-10-25 14:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Shahed Shaikh
In-Reply-To: <1382711917-28501-1-git-send-email-shahed.shaikh@qlogic.com>

From: Shahed Shaikh <shahed.shaikh@qlogic.com>

Only 82xx adapter advertises QLCNIC_FW_CAPABILITY_MORE_CAPS bit.
Reading this bit from 83xx adapter causes the driver to skip
extra capabilities registers.

Because of this, driver was not issuing qlcnic_fw_cmd_set_drv_version()
for 83xx adapter.

This bug was introduced in commit 8af3f33db05c6d0146ad14905145a5c923770856
 ("qlcnic: Add support for 'set driver version' in 83XX").

Signed-off-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 6 +++---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c    | 6 ++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 3ca00e0..ace217c 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -2276,9 +2276,9 @@ int qlcnic_83xx_get_nic_info(struct qlcnic_adapter *adapter,
 		temp = (cmd.rsp.arg[8] & 0x7FFE0000) >> 17;
 		npar_info->max_linkspeed_reg_offset = temp;
 	}
-	if (npar_info->capabilities & QLCNIC_FW_CAPABILITY_MORE_CAPS)
-		memcpy(ahw->extra_capability, &cmd.rsp.arg[16],
-		       sizeof(ahw->extra_capability));
+
+	memcpy(ahw->extra_capability, &cmd.rsp.arg[16],
+	       sizeof(ahw->extra_capability));
 
 out:
 	qlcnic_free_mbx_args(&cmd);
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 9e61eb8..d8f4897 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -1131,7 +1131,10 @@ qlcnic_initialize_nic(struct qlcnic_adapter *adapter)
 		if (err == -EIO)
 			return err;
 		adapter->ahw->extra_capability[0] = temp;
+	} else {
+		adapter->ahw->extra_capability[0] = 0;
 	}
+
 	adapter->ahw->max_mac_filters = nic_info.max_mac_filters;
 	adapter->ahw->max_mtu = nic_info.max_mtu;
 
@@ -2159,8 +2162,7 @@ void qlcnic_set_drv_version(struct qlcnic_adapter *adapter)
 	else if (qlcnic_83xx_check(adapter))
 		fw_cmd = QLCNIC_CMD_83XX_SET_DRV_VER;
 
-	if ((ahw->capabilities & QLCNIC_FW_CAPABILITY_MORE_CAPS) &&
-	    (ahw->extra_capability[0] & QLCNIC_FW_CAPABILITY_SET_DRV_VER))
+	if (ahw->extra_capability[0] & QLCNIC_FW_CAPABILITY_SET_DRV_VER)
 		qlcnic_fw_cmd_set_drv_version(adapter, fw_cmd);
 }
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net 1/2] qlcnic: Do not force adapter to perform LRO without destination IP check
From: Shahed Shaikh @ 2013-10-25 14:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Shahed Shaikh
In-Reply-To: <1382711917-28501-1-git-send-email-shahed.shaikh@qlogic.com>

From: Shahed Shaikh <shahed.shaikh@qlogic.com>

Forcing adapter to perform LRO without destination IP check
degrades the performance.

Signed-off-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
index f8adc7b..b64e2be 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c
@@ -785,8 +785,6 @@ void qlcnic_82xx_config_intr_coalesce(struct qlcnic_adapter *adapter)
 
 #define QLCNIC_ENABLE_IPV4_LRO		1
 #define QLCNIC_ENABLE_IPV6_LRO		2
-#define QLCNIC_NO_DEST_IPV4_CHECK	(1 << 8)
-#define QLCNIC_NO_DEST_IPV6_CHECK	(2 << 8)
 
 int qlcnic_82xx_config_hw_lro(struct qlcnic_adapter *adapter, int enable)
 {
@@ -806,11 +804,10 @@ int qlcnic_82xx_config_hw_lro(struct qlcnic_adapter *adapter, int enable)
 
 	word = 0;
 	if (enable) {
-		word = QLCNIC_ENABLE_IPV4_LRO | QLCNIC_NO_DEST_IPV4_CHECK;
+		word = QLCNIC_ENABLE_IPV4_LRO;
 		if (adapter->ahw->extra_capability[0] &
 		    QLCNIC_FW_CAP2_HW_LRO_IPV6)
-			word |= QLCNIC_ENABLE_IPV6_LRO |
-				QLCNIC_NO_DEST_IPV6_CHECK;
+			word |= QLCNIC_ENABLE_IPV6_LRO;
 	}
 
 	req.words[0] = cpu_to_le64(word);
-- 
1.8.1.4

^ permalink raw reply related

* RE: linux-next: manual merge of the arm tree
From: Dmitry Kravkov @ 2013-10-25 15:20 UTC (permalink / raw)
  To: Thierry Reding, Russell King, Merav Sicron, David Miller,
	netdev@vger.kernel.org
  Cc: linux-next@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1382454517-4074-2-git-send-email-treding@nvidia.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Thierry Reding
> Sent: Tuesday, October 22, 2013 6:09 PM
> To: Russell King; Merav Sicron; David Miller; netdev@vger.kernel.org
> Cc: linux-next@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: linux-next: manual merge of the arm tree
> 
> Today's linux-next merge of the arm tree got a conflict in
> 
> 	drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> 
> caused by commits 1bfa2c4 (DMA-API: net: broadcom/bnx2x: replace
> dma_set_mask()+dma_set_coherent_mask() with new helper) and edd3147
> (bnx2x: Set NETIF_F_HIGHDMA unconditionally).
> 
> I fixed it up (see below). Please verify that the resolution looks good.
> 
> Thanks,
> Thierry
> ---
> diff --cc drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index b42f89c,38bf998..767aafb
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@@ -12117,12 -12079,9 +12117,8 @@@ static int
> bnx2x_set_coherency_mask(str
>   {
>   	struct device *dev = &bp->pdev->dev;
> 
> - 	if (dma_set_mask(dev, DMA_BIT_MASK(64)) == 0) {
> - 		if (dma_set_coherent_mask(dev, DMA_BIT_MASK(64)) != 0) {
> - 			dev_err(dev, "dma_set_coherent_mask failed,
> aborting\n");
> - 			return -EIO;
> - 		}
> - 	} else if (dma_set_mask(dev, DMA_BIT_MASK(32)) != 0) {
>  -	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) == 0) {
>  -		bp->flags |= USING_DAC_FLAG;
>  -	} else if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)) !=
> 0) {
> ++	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) != 0 &&
> ++	    dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)) != 0) {
>   		dev_err(dev, "System does not support DMA, aborting\n");
>   		return -EIO;
>   	}

The fix is correct. Thanks, Thierry.
Acked-by: Dmitry Kravkov <dmitry@broadcom.com>
 

^ permalink raw reply

* Re: PROBLEM: GRE forwarding not working with 3.10.x, WCCPv2 and Cisco 7200 Router showing IP0 bad-hlen 4 in tcpdump
From: Pravin Shelar @ 2013-10-25 15:25 UTC (permalink / raw)
  To: Peter Schmitt; +Cc: Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <1382096356.63655.YahooMailNeo@web172002.mail.ir2.yahoo.com>

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

Hi Peter,
I think its easy to fix this case, Can you try attached patch?
If it works I will send formal patch for stable.

Thanks,
Pravin.

On Fri, Oct 18, 2013 at 4:39 AM, Peter Schmitt <peter.schmitt82@yahoo.de> wrote:
> Hi,
>
>
>
>> On Thursday, October 17, 2013 3:44 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > On Thu, 2013-10-17 at 11:53 +0100, Peter Schmitt wrote:
>>>  Hi,
>>>
>>>
>>>  >
>>>  > 3.10 is buggy (for ETH_P_WCCP support I mean), 3.11 has the probable
>>>  > fix :
>>>  >
>>>  > commit 3d7b46cd20e300bd6989fb1f43d46f1b9645816e
>>>  > ("ip_tunnel: push generic protocol handling to ip_tunnel
>> module.")
>>>  >
>>>  > The problem is 3.10 do not pull the extra 4 bytes of WCCP
>>>  >
>>>  > This is now properly done in iptunnel_pull_header()
>>>  >
>>>
>>>  First of all, many thanks for your help on this.
>>>
>>>  I tried to apply the fix to the 3.10.16 sources, as I would like to
>>>  stay with the long-term line. Unfortunately it does not apply, as
>>>  there are a lot of dependencies on other patches.
>>>
>>>  Do you think there will be a fix for the long-time 3.10 kernel line?
>>>  Or can you guide me on how to apply this fix to the long-term kernel?
>>
>> 3.10 is right in the middle of GRE refactoring, and many bugs are in it.
>>
>> It might be very painful to get a complete list of patches to backport.
>
> thanks again. Yes I saw that there were lots of changes.
>
> Do you think there will be a fix for all the 3.10.x stable long term kernel users? Many of them might not be able to use some newer kernel easily or will this be a "won't fix"?
>
> Best regards,
> Peter
>

[-- Attachment #2: 0001-ip_gre-Fix-WCCPv2-header-parsing.patch --]
[-- Type: text/x-patch, Size: 2787 bytes --]

From 60e0148f424a8bee987f8c606c4ac23cbe70a567 Mon Sep 17 00:00:00 2001
From: Pravin B Shelar <pshelar@nicira.com>
Date: Fri, 25 Oct 2013 08:19:59 -0700
Subject: [PATCH] ip_gre: Fix WCCPv2 header parsing.

Pull currect header-len for gre packet.
---
 include/net/ip_tunnels.h |    2 +-
 net/ipv4/ip_gre.c        |    2 +-
 net/ipv4/ip_tunnel.c     |    4 ++--
 net/ipv4/ipip.c          |    2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 09b1360..79cf958 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -113,7 +113,7 @@ struct ip_tunnel *ip_tunnel_lookup(struct ip_tunnel_net *itn,
 				   __be32 key);
 
 int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
-		  const struct tnl_ptk_info *tpi, bool log_ecn_error);
+		  const struct tnl_ptk_info *tpi, int hdr_len, bool log_ecn_error);
 int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 			 struct ip_tunnel_parm *p);
 int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 855004f..3ddc8df 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -335,7 +335,7 @@ static int ipgre_rcv(struct sk_buff *skb)
 				  iph->saddr, iph->daddr, tpi.key);
 
 	if (tunnel) {
-		ip_tunnel_rcv(tunnel, skb, &tpi, log_ecn_error);
+		ip_tunnel_rcv(tunnel, skb, &tpi, hdr_len, log_ecn_error);
 		return 0;
 	}
 	icmp_send(skb, ICMP_DEST_UNREACH, ICMP_PORT_UNREACH, 0);
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index cbfc37f..49fb608 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -402,7 +402,7 @@ static struct ip_tunnel *ip_tunnel_create(struct net *net,
 }
 
 int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
-		  const struct tnl_ptk_info *tpi, bool log_ecn_error)
+		  const struct tnl_ptk_info *tpi, int hdr_len, bool log_ecn_error)
 {
 	struct pcpu_tstats *tstats;
 	const struct iphdr *iph = ip_hdr(skb);
@@ -413,7 +413,7 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
 	skb->protocol = tpi->proto;
 
 	skb->mac_header = skb->network_header;
-	__pskb_pull(skb, tunnel->hlen);
+	__pskb_pull(skb, hdr_len);
 	skb_postpull_rcsum(skb, skb_transport_header(skb), tunnel->hlen);
 #ifdef CONFIG_NET_IPGRE_BROADCAST
 	if (ipv4_is_multicast(iph->daddr)) {
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 7cfc456..f5cc7b3 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -195,7 +195,7 @@ static int ipip_rcv(struct sk_buff *skb)
 	if (tunnel) {
 		if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
 			goto drop;
-		return ip_tunnel_rcv(tunnel, skb, &tpi, log_ecn_error);
+		return ip_tunnel_rcv(tunnel, skb, &tpi, 0, log_ecn_error);
 	}
 
 	return -1;
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 1/3] vxlan: silence one build warning
From: Stephen Hemminger @ 2013-10-25 15:41 UTC (permalink / raw)
  To: Zhi Yong Wu; +Cc: netdev, linux-kernel, Zhi Yong Wu
In-Reply-To: <1382687360-27794-1-git-send-email-zwu.kernel@gmail.com>

On Fri, 25 Oct 2013 15:49:18 +0800
Zhi Yong Wu <zwu.kernel@gmail.com> wrote:

> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> 
> drivers/net/vxlan.c: In function ‘vxlan_sock_add’:
> drivers/net/vxlan.c:2298:11: warning: ‘sock’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/net/vxlan.c:2275:17: note: ‘sock’ was declared here
>   LD      drivers/net/built-in.o
> 
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> ---
>  drivers/net/vxlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 2ef5b62..e15f1af 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2272,7 +2272,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
>  {
>  	struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>  	struct vxlan_sock *vs;
> -	struct socket *sock;
> +	struct socket *sock = NULL;
>  	struct sock *sk;
>  	int rc = 0;
>  	unsigned int h;

This only happens with certain versions of Gcc which have buggy dependency
analysis. It doesn't happen with Gcc 4.7, think it only shows up in 4.4.
I would rather not fix the warning this way since it risks masking later bugs if this code ever changes.

^ permalink raw reply

* Re: [PATCH v5] net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)
From: Arvid Brodin @ 2013-10-25 18:20 UTC (permalink / raw)
  To: Joe Perches
  Cc: netdev@vger.kernel.org, David Miller, Stephen Hemminger,
	Javier Boticario, balferreira@googlemail.com,
	Elías Molina Muñoz
In-Reply-To: <1382547142.22433.18.camel@joe-AO722>

On 2013-10-23 18:52, Joe Perches wrote:
> On Wed, 2013-10-23 at 18:09 +0200, Arvid Brodin wrote:
>> High-availability Seamless Redundancy ("HSR") provides instant failover
>> redundancy for Ethernet networks. It requires a special network topology where
>> all nodes are connected in a ring (each node having two physical network
>> interfaces). It is suited for applications that demand high availability and
>> very short reaction time.
> 
> trivia: (can be ignored/fixed later)
> 
>> +static void restore_slaves(struct net_device *hsr_dev)
>> +{
>> +	struct hsr_priv *hsr_priv;
>> +	int i;
>> +	int res;
>> +
>> +	hsr_priv = netdev_priv(hsr_dev);
>> +
>> +	rtnl_lock();
>> +
>> +	/* Restore promiscuity */
>> +	for (i = 0; i < 2; i++) {
> 
> I presume all of these for slave loops that use
> for (i = 0; i < 2; i++) should be i < HSR_MAX_SLAVE

Yes, that's not nice at all. Will fix.
 
> Maybe it'd be useful to add a foreach_slave() helper.

I experimented a bit with this, but since the slaves are held in an array and
can be NULL, and since we aren't allowed to do for loop initial declarations,
it became a bit cumbersome. So I'll stick with the "manual" for loops.

 
>> +static struct node_entry *find_node_by_AddrA(struct list_head *node_db,
>> +					     const unsigned char addr[ETH_ALEN])
>> +{
>> +	struct node_entry *node;
>> +
>> +	list_for_each_entry_rcu(node, node_db, mac_list) {
>> +		if (!compare_ether_addr(node->MacAddressA, addr))
> 
> Please use ether_addr_equal instead for all these uses.
> compare_ether_addr should be removed one day.

Ok! Much nicer with ether_addr_equal().


>> +static struct node_entry *find_node_by_AddrB(struct list_head *node_db,
>> +					     const unsigned char addr[ETH_ALEN])
> []
>> +		if (!compare_ether_addr(node->MacAddressB, addr))
> []
>> +struct node_entry *hsr_find_node(struct list_head *node_db, struct sk_buff *skb)
> []
>> +		if (!compare_ether_addr(node->MacAddressA, ethhdr->h_source))
>> +			return node;
>> +		if (!compare_ether_addr(node->MacAddressB, ethhdr->h_source))
>> +			return node;
> 
>> +struct node_entry *hsr_merge_node(struct hsr_priv *hsr_priv,
>> +				  struct node_entry *node,
>> +				  struct sk_buff *skb,
>> +				  enum hsr_dev_idx dev_idx)
> []
>> +	if (node && compare_ether_addr(node->MacAddressA, hsr_sp->MacAddressA)) {
> []
>> +	if (node && (dev_idx == node->AddrB_if) &&
>> +	    compare_ether_addr(node->MacAddressB, hsr_ethsup->ethhdr.h_source)) {
> []
>> +	if (node && (dev_idx != node->AddrB_if) &&
>> +	    (node->AddrB_if != HSR_DEV_NONE) &&
>> +	    compare_ether_addr(node->MacAddressA, hsr_ethsup->ethhdr.h_source)) {
> []
>> +	if (compare_ether_addr(hsr_sp->MacAddressA, hsr_ethsup->ethhdr.h_source))
> 
> []
> 
>> +/* above(a, b) - return 1 if a > b, 0 otherwise.
>> + */
>> +static bool above(u16 a, u16 b)
>> +{
>> +	/* Remove inconsistency where above(a, b) == below(a, b) */
>> +	if ((int) b - a == 32768)
>> +		return 0;
>> +
>> +	return (((s16) (b - a)) < 0);
>> +}
>> +#define below(a, b)		above((b), (a))
>> +#define above_or_eq(a, b)	(!below((a), (b)))
>> +#define below_or_eq(a, b)	(!above((a), (b)))
> 
> This looks odd. 

It relies on unsigned arithmetic to compare two values that may wrap. I.e.,
it doesn't care about the absolute sizes, but only about the distance 
between the numbers. 

It is inspired in part by the code in jiffies.h, but adapted to 16-bit
types. The code you suggested (below) will not work in this case.

See e.g. http://en.wikipedia.org/wiki/Serial_number_arithmetic


> 
> static bool above(u16 a, u16 b)
> {
> 	return a > b;
> }
> #define below(a, b) above(b, a)
> 
> static bool above_or_eq(u16 a, u16 b)
> {
> 	return a >= b;
> }
> #define below_or_eq(a, b) above_or_eq(b, a)
> 
> 
> 


-- 
Arvid Brodin | Consultant (Linux)
XDIN AB | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden | xdin.com

^ permalink raw reply

* Re: [PATCH v5] net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)
From: Joe Perches @ 2013-10-25 18:31 UTC (permalink / raw)
  To: Arvid Brodin
  Cc: netdev@vger.kernel.org, David Miller, Stephen Hemminger,
	Javier Boticario, balferreira@googlemail.com,
	Elías Molina Muñoz
In-Reply-To: <526AB665.5020402@xdin.com>

On Fri, 2013-10-25 at 20:20 +0200, Arvid Brodin wrote:
> On 2013-10-23 18:52, Joe Perches wrote:
> > On Wed, 2013-10-23 at 18:09 +0200, Arvid Brodin wrote:
[]
> >> +/* above(a, b) - return 1 if a > b, 0 otherwise.
> >> + */
> >> +static bool above(u16 a, u16 b)
> >> +{
> >> +	/* Remove inconsistency where above(a, b) == below(a, b) */
> >> +	if ((int) b - a == 32768)
> >> +		return 0;
> >> +
> >> +	return (((s16) (b - a)) < 0);
> >> +}
> >> +#define below(a, b)		above((b), (a))
> >> +#define above_or_eq(a, b)	(!below((a), (b)))
> >> +#define below_or_eq(a, b)	(!above((a), (b)))
> > 
> > This looks odd. 
> 
> It relies on unsigned arithmetic to compare two values that may wrap. I.e.,
> it doesn't care about the absolute sizes, but only about the distance 
> between the numbers. 
> 
> It is inspired in part by the code in jiffies.h, but adapted to 16-bit
> types. The code you suggested (below) will not work in this case.
> 
> See e.g. http://en.wikipedia.org/wiki/Serial_number_arithmetic

No worries, I was just reading the comment as the
comment doesn't match the code.

Perhaps the comment should be updated to reflect the
wrapping test.

^ permalink raw reply

* Re: [PATCH] Documentation/networking: netdev-FAQ typo corrections
From: Paul Gortmaker @ 2013-10-25 18:56 UTC (permalink / raw)
  To: Randy Dunlap, netdev@vger.kernel.org, David Miller
In-Reply-To: <5269CFE9.3020703@infradead.org>

On 13-10-24 09:56 PM, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> 
> Various typo fixes to netdev-FAQ.txt:
> - capitalize Linux
> - hyphenate dual-word adjectives
> - minor punctuation fixes
> 
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>

I've no objections to those kinds of changes.

Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>

P.
--

> ---
>  Documentation/networking/netdev-FAQ.txt |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> --- lnx-312-rc6.orig/Documentation/networking/netdev-FAQ.txt
> +++ lnx-312-rc6/Documentation/networking/netdev-FAQ.txt
> @@ -4,23 +4,23 @@ Information you need to know about netde
>  
>  Q: What is netdev?
>  
> -A: It is a mailing list for all network related linux stuff.  This includes
> +A: It is a mailing list for all network-related Linux stuff.  This includes
>     anything found under net/  (i.e. core code like IPv6) and drivers/net
> -   (i.e. hardware specific drivers) in the linux source tree.
> +   (i.e. hardware specific drivers) in the Linux source tree.
>  
>     Note that some subsystems (e.g. wireless drivers) which have a high volume
>     of traffic have their own specific mailing lists.
>  
> -   The netdev list is managed (like many other linux mailing lists) through
> +   The netdev list is managed (like many other Linux mailing lists) through
>     VGER ( http://vger.kernel.org/ ) and archives can be found below:
>  
>  	http://marc.info/?l=linux-netdev
>  	http://www.spinics.net/lists/netdev/
>  
> -   Aside from subsystems like that mentioned above, all network related linux
> -   development (i.e. RFC, review, comments, etc) takes place on netdev.
> +   Aside from subsystems like that mentioned above, all network-related Linux
> +   development (i.e. RFC, review, comments, etc.) takes place on netdev.
>  
> -Q: How do the changes posted to netdev make their way into linux?
> +Q: How do the changes posted to netdev make their way into Linux?
>  
>  A: There are always two trees (git repositories) in play.  Both are driven
>     by David Miller, the main network maintainer.  There is the "net" tree,
> @@ -35,7 +35,7 @@ A: There are always two trees (git repos
>  Q: How often do changes from these trees make it to the mainline Linus tree?
>  
>  A: To understand this, you need to know a bit of background information
> -   on the cadence of linux development.  Each new release starts off with
> +   on the cadence of Linux development.  Each new release starts off with
>     a two week "merge window" where the main maintainers feed their new
>     stuff to Linus for merging into the mainline tree.  After the two weeks,
>     the merge window is closed, and it is called/tagged "-rc1".  No new
> @@ -46,7 +46,7 @@ A: To understand this, you need to know
>     things are in a state of churn), and a week after the last vX.Y-rcN
>     was done, the official "vX.Y" is released.
>  
> -   Relating that to netdev:  At the beginning of the 2 week merge window,
> +   Relating that to netdev:  At the beginning of the 2-week merge window,
>     the net-next tree will be closed - no new changes/features.  The
>     accumulated new content of the past ~10 weeks will be passed onto
>     mainline/Linus via a pull request for vX.Y -- at the same time,
> @@ -59,12 +59,12 @@ A: To understand this, you need to know
>     IMPORTANT:  Do not send new net-next content to netdev during the
>     period during which net-next tree is closed.
>  
> -   Shortly after the two weeks have passed, (and vX.Y-rc1 is released) the
> +   Shortly after the two weeks have passed (and vX.Y-rc1 is released), the
>     tree for net-next reopens to collect content for the next (vX.Y+1) release.
>  
>     If you aren't subscribed to netdev and/or are simply unsure if net-next
>     has re-opened yet, simply check the net-next git repository link above for
> -   any new networking related commits.
> +   any new networking-related commits.
>  
>     The "net" tree continues to collect fixes for the vX.Y content, and
>     is fed back to Linus at regular (~weekly) intervals.  Meaning that the
> @@ -217,7 +217,7 @@ A: Attention to detail.  Re-read your ow
>     to why it happens, and then if necessary, explain why the fix proposed
>     is the best way to get things done.   Don't mangle whitespace, and as
>     is common, don't mis-indent function arguments that span multiple lines.
> -   If it is your 1st patch, mail it to yourself so you can test apply
> +   If it is your first patch, mail it to yourself so you can test apply
>     it to an unpatched tree to confirm infrastructure didn't mangle it.
>  
>     Finally, go back and read Documentation/SubmittingPatches to be
> 

^ permalink raw reply

* Re: [PATCH v2] can: add Renesas R-Car CAN driver
From: Wolfgang Grandegger @ 2013-10-25 19:28 UTC (permalink / raw)
  To: Sergei Shtylyov, netdev, mkl, linux-can; +Cc: linux-sh, vksavl
In-Reply-To: <201310250303.00695.sergei.shtylyov@cogentembedded.com>

On 10/25/2013 01:03 AM, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs. 
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> The patch is against the 'linux-can-next.git' repo.
> 
> Changes in version 2:
> - added function to clean up TX mailboxes after bus error and bus-off;
> - added module parameter to enable hardware recovery from bus-off, added handler
>   for the bus-off recovery interrupt, and set the control register according to
>   the parameter value and the restart timer setting in rcar_can_start();
> - changed the way CAN_ERR_CRTL_[RT]X_{PASSIVE|WARNING} flags are set to a more
>   realicstic one;
> - replaced MBX_* macros and rcar_can_mbx_{read|write}[bl]() functions with
>   'struct rcar_can_mbox_regs', 'struct rcar_can_regs', and {read|write}[bl](),
>   replaced 'reg_base' field of 'struct rcar_can_priv' with 'struct rcar_can_regs
>   __iomem *regs';
> - added 'ier' field to 'struct rcar_can_priv' to cache the current value of the
>   interrupt enable register;
> - added a check for enabled interrupts on entry to rcar_can_interrupt();
> - limited transmit mailbox search loop in rcar_can_interrupt();
> - decoupled  TX byte count increment from can_get_echo_skb() call;
> - removed netif_queue_stopped() call from rcar_can_interrupt();
> - added clk_prepare_enable()/clk_disable_unprepare() to ndo_{open|close}(),
>   do_set_bittiming(), and do_get_berr_counter() methods, removed clk_enable()
>   call from the probe() method and clk_disable() call from the remove() method;
> - allowed rcar_can_set_bittiming() to be called when the device is closed and
>   remove  explicit call to it from rcar_can_start();
> - switched to using mailbox number priority transmit mode, and switched to the
>   sequential mailbox use in ndo_start_xmit() method;
> - stopped reading the message control registers in ndo_start_xmit() method;
> - avoided returning NETDEV_TX_BUSY from ndo_start_xmit() method;
> - stopped reading data when RTR bit is set in the CAN frame;
> - made 'num_pkts' variable *int* and moved its check to the *while* condition in
>   rcar_can_rx_poll();
> - used dev_get_platdata() in the probe() method;
> - enabled bus error interrupt only if CAN_CTRLMODE_BERR_REPORTING flag is set;
> - started reporting CAN_CTRLMODE_BERR_REPORTING support and stopped reporting
>   CAN_CTRLMODE_3_SAMPLES support;
> - set CAN_ERR_ACK flag on ACK error;
> - switched to incrementing bus error counter only once per bus error interrupt;
> - started switching to CAN sleep mode in rcar_can_stop() and stopped switching
>   to it in the remove() method;
> - removed netdev_err() calls on allocation failure in rcar_can_error() and
>   rcar_can_rx_pkt();
> - removed "CANi" from the register offset macro comments.
> 
>  drivers/net/can/Kconfig               |    9 
>  drivers/net/can/Makefile              |    1 
>  drivers/net/can/rcar_can.c            |  920 ++++++++++++++++++++++++++++++++++
>  include/linux/can/platform/rcar_can.h |   15 
>  4 files changed, 945 insertions(+)
> 
> Index: linux-can-next/drivers/net/can/Kconfig
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Kconfig
> +++ linux-can-next/drivers/net/can/Kconfig
> @@ -125,6 +125,15 @@ config CAN_GRCAN
>  	  endian syntheses of the cores would need some modifications on
>  	  the hardware level to work.
>  
> +config CAN_RCAR
> +	tristate "Renesas R-Car CAN controller"
> +	---help---
> +	  Say Y here if you want to use CAN controller found on Renesas R-Car
> +	  SoCs.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called rcar_can.
> +
>  source "drivers/net/can/mscan/Kconfig"
>  
>  source "drivers/net/can/sja1000/Kconfig"
> Index: linux-can-next/drivers/net/can/Makefile
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Makefile
> +++ linux-can-next/drivers/net/can/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ica
>  obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>  obj-$(CONFIG_PCH_CAN)		+= pch_can.o
>  obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
> +obj-$(CONFIG_CAN_RCAR)		+= rcar_can.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> Index: linux-can-next/drivers/net/can/rcar_can.c
> ===================================================================
> --- /dev/null
> +++ linux-can-next/drivers/net/can/rcar_can.c
> @@ -0,0 +1,920 @@
> +/*
> + * Renesas R-Car CAN device driver
> + *
> + * Copyright (C) 2013 Cogent Embedded, Inc. <source@cogentembedded.com>
> + * Copyright (C) 2013 Renesas Solutions Corp.
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/errno.h>
> +#include <linux/netdevice.h>
> +#include <linux/platform_device.h>
> +#include <linux/can/led.h>
> +#include <linux/can/dev.h>
> +#include <linux/clk.h>
> +#include <linux/can/platform/rcar_can.h>
> +
> +#define DRV_NAME	"rcar_can"
> +
> +#define RCAR_CAN_MIER1	0x42C	/* Mailbox Interrupt Enable Register 1 */
> +#define RCAR_CAN_MKR(n)	((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
> +				/* Mask Register */
> +#define RCAR_CAN_MKIVLR0 0x438	/* Mask Invalid Register 0 */
> +#define RCAR_CAN_MIER0	 0x43C	/* Mailbox Interrupt Enable Register 0 */
> +
> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* Message Control Register */
> +#define RCAR_CAN_CTLR	0x840	/* Control Register */
> +#define RCAR_CAN_STR	0x842	/* Status Register */
> +#define RCAR_CAN_BCR	0x844	/* Bit Configuration Register */
> +#define RCAR_CAN_CLKR	0x847	/* Clock Select Register */
> +#define RCAR_CAN_EIER	0x84C	/* Error Interrupt Enable Register */
> +#define RCAR_CAN_EIFR	0x84D	/* Err Interrupt Factor Judge Register */
> +#define RCAR_CAN_RECR	0x84E	/* Receive Error Count Register */
> +#define RCAR_CAN_TECR	0x84F	/* Transmit Error Count Register */
> +#define RCAR_CAN_ECSR	0x850	/* Error Code Store Register */
> +#define RCAR_CAN_MSSR	0x852	/* Mailbox Search Status Register */
> +#define RCAR_CAN_MSMR	0x853	/* Mailbox Search Mode Register */
> +#define RCAR_CAN_TCR	0x858	/* Test Control Register */
> +#define RCAR_CAN_IER	0x860	/* Interrupt Enable Register */
> +#define RCAR_CAN_ISR	0x861	/* Interrupt Status Register */
> +
> +/* Control Register bits */
> +#define CTLR_BOM	(3 << 11) /* Bus-Off Recovery Mode Bits */
> +#define CTLR_BOM_ENT	BIT(11)	/* Entry to halt mode at bus-off entry */
> +#define CTLR_SLPM	BIT(10)
> +#define CTLR_HALT	BIT(9)
> +#define CTLR_RESET	BIT(8)
> +#define CTLR_FORCE_RESET (3 << 8)
> +#define CTLR_TPM	BIT(4)	/* Transmission Priority Mode Select Bit */
> +#define CTLR_IDFM_MIXED	BIT(2)	/* Mixed ID mode */
> +
> +/* Message Control Register bits */
> +#define MCTL_TRMREQ	BIT(7)
> +#define MCTL_RECREQ	BIT(6)
> +#define MCTL_ONESHOT	BIT(4)
> +#define MCTL_SENTDATA	BIT(0)
> +#define MCTL_NEWDATA	BIT(0)
> +
> +#define N_RX_MKREGS	2	/* Number of mask registers */
> +				/* for Rx mailboxes 0-31 */
> +
> +/* Bit Configuration Register settings */
> +#define BCR_TSEG1(x)	(((x) & 0x0f) << 28)
> +#define BCR_BPR(x)	(((x) & 0x3ff) << 16)
> +#define BCR_SJW(x)	(((x) & 0x3) << 12)
> +#define BCR_TSEG2(x)	(((x) & 0x07) << 8)
> +
> +/* Mailbox and Mask Registers bits */
> +#define RCAR_CAN_IDE	BIT(31)
> +#define RCAR_CAN_RTR	BIT(30)
> +#define RCAR_CAN_SID_SHIFT 18
> +
> +/* Interrupt Enable Register bits */
> +#define IER_ERSIE	BIT(5)	/* Error (ERS) Interrupt Enable Bit */
> +#define IER_RXM0IE	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_RXM1IE	BIT(1)	/* Mailbox 1 Successful Reception (RXM0) */
> +				/* Interrupt Enable Bit */
> +#define IER_TXMIE	BIT(0)	/* Mailbox 32 to 63 Successful Tx */
> +				/* Interrupt Enable Bit */
> +
> +/* Interrupt Status Register bits */
> +#define ISR_ERSF	BIT(5)	/* Error (ERS) Interrupt Status Bit */
> +#define ISR_RXM0F	BIT(2)	/* Mailbox 0 Successful Reception (RXM0) */
> +				/* Interrupt Status Bit */
> +#define ISR_RXM1F	BIT(1)	/* Mailbox 1 to 63 Successful Reception */
> +				/* (RXM1) Interrupt Status Bit */
> +#define ISR_TXMF	BIT(0)	/* Mailbox 32 to 63 Successful Transmission */
> +				/* (TXM) Interrupt Status Bit */
> +
> +/* Error Interrupt Enable Register bits */
> +#define EIER_BLIE	BIT(7)	/* Bus Lock Interrupt Enable */
> +#define EIER_OLIE	BIT(6)	/* Overload Frame Transmit Interrupt Enable */
> +#define EIER_ORIE	BIT(5)	/* Receive Overrun Interrupt Enable */
> +#define EIER_BORIE	BIT(4)	/* Bus-Off Recovery Interrupt Enable */
> +
> +#define EIER_BOEIE	BIT(3)	/* Bus-Off Entry Interrupt Enable */
> +#define EIER_EPIE	BIT(2)	/* Error Passive Interrupt Enable */
> +#define EIER_EWIE	BIT(1)	/* Error Warning Interrupt Enable */
> +#define EIER_BEIE	BIT(0)	/* Bus Error Interrupt Enable */
> +
> +/* Error Interrupt Factor Judge Register bits */
> +#define EIFR_BLIF	BIT(7)	/* Bus Lock Detect Flag */
> +#define EIFR_OLIF	BIT(6)	/* Overload Frame Transmission Detect Flag */
> +#define EIFR_ORIF	BIT(5)	/* Receive Overrun Detect Flag */
> +#define EIFR_BORIF	BIT(4)	/* Bus-Off Recovery Detect Flag */
> +#define EIFR_BOEIF	BIT(3)	/* Bus-Off Entry Detect Flag */
> +#define EIFR_EPIF	BIT(2)	/* Error Passive Detect Flag */
> +#define EIFR_EWIF	BIT(1)	/* Error Warning Detect Flag */
> +#define EIFR_BEIF	BIT(0)	/* Bus Error Detect Flag */
> +
> +/* Error Code Store Register bits */
> +#define ECSR_EDPM	BIT(7)	/* Error Display Mode Select Bit */
> +#define ECSR_ADEF	BIT(6)	/* ACK Delimiter Error Flag */
> +#define ECSR_BE0F	BIT(5)	/* Bit Error (dominant) Flag */
> +#define ECSR_BE1F	BIT(4)	/* Bit Error (recessive) Flag */
> +#define ECSR_CEF	BIT(3)	/* CRC Error Flag */
> +#define ECSR_AEF	BIT(2)	/* ACK Error Flag */
> +#define ECSR_FEF	BIT(1)	/* Form Error Flag */
> +#define ECSR_SEF	BIT(0)	/* Stuff Error Flag */
> +
> +/* Mailbox Search Status Register bits */
> +#define MSSR_SEST	BIT(7)	/* Search Result Status Bit */
> +#define MSSR_MBNST	0x3f	/* Search Result Mailbox Number Status mask */
> +
> +/* Mailbox Search Mode Register values */
> +#define MSMR_TXMB	1	/* Transmit mailbox search mode */
> +#define MSMR_RXMB	0	/* Receive mailbox search mode */
> +
> +/* Mailbox configuration:
> + * mailbox 0 - not used
> + * mailbox 1-31 - Rx
> + * mailbox 32-63 - Tx
> + * no FIFO mailboxes
> + */
> +#define N_MBX		64
> +#define FIRST_TX_MB	32
> +#define N_TX_MB		(N_MBX - FIRST_TX_MB)
> +#define RX_MBX_MASK	0xFFFFFFFE
> +
> +#define RCAR_CAN_NAPI_WEIGHT (FIRST_TX_MB - 1)
> +
> +static bool autorecovery;
> +module_param(autorecovery, bool, 0644);
> +MODULE_PARM_DESC(autorecovery, "Automatic hardware recovery from bus-off");

Software recovery is the preferred solution. No need to support
automatic recovery by the hardware.

> +
> +/* Mailbox registers structure */
> +struct rcar_can_mbox_regs {
> +	u32 id;		/* IDE and RTR bits, SID and EID */
> +	u8 stub;	/* Not used */
> +	u8 dlc;		/* Data Length Code - bits [0..3] */
> +	u8 data[8];	/* Data Bytes */
> +	u8 tsh;		/* Time Stamp Higher Byte */
> +	u8 tsl;		/* Time Stamp Lower Byte */

I would add padding bytes here to ensure alignment.

> +};
> +
> +struct rcar_can_regs {
> +	struct rcar_can_mbox_regs mb[N_MBX];
> +};

Where are the other registers? Using a mix of struct and #define offsets
is really ugly.

> +struct rcar_can_priv {
> +	struct can_priv can;	/* Must be the first member! */
> +	struct net_device *ndev;
> +	struct napi_struct napi;
> +	struct rcar_can_regs __iomem *regs;
> +	struct clk *clk;
> +	spinlock_t mier_lock;
> +	u8 clock_select;
> +	u8 ier;

s/ier/ier_shadow/ ?

> +};
> +
> +static const struct can_bittiming_const rcar_can_bittiming_const = {
> +	.name = DRV_NAME,
> +	.tseg1_min = 4,
> +	.tseg1_max = 16,
> +	.tseg2_min = 2,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 1024,
> +	.brp_inc = 1,
> +};
> +
> +static inline u32 rcar_can_readl(struct rcar_can_priv *priv, int reg)
> +{
> +	return readl((void __iomem *)priv->regs + reg);
> +}
> +
> +static inline u16 rcar_can_readw(struct rcar_can_priv *priv, int reg)
> +{
> +	return readw((void __iomem *)priv->regs + reg);
> +}
> +
> +static inline u8 rcar_can_readb(struct rcar_can_priv *priv, int reg)
> +{
> +	return readb((void __iomem *)priv->regs + reg);
> +}
> +
> +static inline void rcar_can_writel(struct rcar_can_priv *priv, int reg, u32 val)
> +{
> +	writel(val, (void __iomem *)priv->regs + reg);
> +}
> +
> +static inline void rcar_can_writew(struct rcar_can_priv *priv, int reg, u16 val)
> +{
> +	writew(val, (void __iomem *)priv->regs + reg);
> +}
> +
> +static inline void rcar_can_writeb(struct rcar_can_priv *priv, int reg, u8 val)
> +{
> +	writeb(val, (void __iomem *)priv->regs + reg);
> +}

See my comment above.

> +static void tx_failure_cleanup(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u32 mier1;
> +	u8 mbx;
> +
> +	spin_lock(&priv->mier_lock);
> +	mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +	for (mbx = FIRST_TX_MB; mbx < N_MBX; mbx++) {
> +		if (mier1 & BIT(mbx - FIRST_TX_MB)) {
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), 0);
> +			can_free_echo_skb(ndev, mbx - FIRST_TX_MB);
> +		}
> +	}
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +	spin_unlock(&priv->mier_lock);
> +}
> +
> +static void rcar_can_start(struct net_device *ndev);

Forward declarations should be avoided.

> +
> +static void rcar_can_error(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u8 eifr, txerr = 0, rxerr = 0;
> +
> +	/* Propagate the error condition to the CAN stack */
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb)
> +		return;
> +	eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
> +	if (eifr & (EIFR_EWIF | EIFR_EPIF)) {
> +		cf->can_id |= CAN_ERR_CRTL;
> +		txerr = rcar_can_readb(priv, RCAR_CAN_TECR);
> +		rxerr = rcar_can_readb(priv, RCAR_CAN_RECR);

Could you please add these values to data[6..7] as shown here:

http://lxr.free-electrons.com/source/drivers/net/can/sja1000/sja1000.c#L475

> +	}
> +	if (eifr & EIFR_BEIF) {
> +		int rx_errors = 0, tx_errors = 0;
> +		u8 ecsr;
> +
> +		if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> +			tx_failure_cleanup(ndev);
> +		netdev_dbg(priv->ndev, "Bus error interrupt:\n");
> +		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> +		cf->data[2] = CAN_ERR_PROT_UNSPEC;
> +
> +		ecsr = rcar_can_readb(priv, RCAR_CAN_ECSR);
> +		if (ecsr & ECSR_ADEF) {
> +			netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_ADEF);

Please avoid casts here and below.

> +		}
> +		if (ecsr & ECSR_BE0F) {
> +			netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT0;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE0F);
> +		}
> +		if (ecsr & ECSR_BE1F) {
> +			netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
> +			cf->data[2] |= CAN_ERR_PROT_BIT1;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE1F);
> +		}
> +		if (ecsr & ECSR_CEF) {
> +			netdev_dbg(priv->ndev, "CRC Error\n");
> +			cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_CEF);
> +		}
> +		if (ecsr & ECSR_AEF) {
> +			netdev_dbg(priv->ndev, "ACK Error\n");
> +			cf->can_id |= CAN_ERR_ACK;
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> +			tx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_AEF);
> +		}
> +		if (ecsr & ECSR_FEF) {
> +			netdev_dbg(priv->ndev, "Form Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_FORM;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_FEF);
> +		}
> +		if (ecsr & ECSR_SEF) {
> +			netdev_dbg(priv->ndev, "Stuff Error\n");
> +			cf->data[2] |= CAN_ERR_PROT_STUFF;
> +			rx_errors++;
> +			rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_SEF);
> +		}
> +
> +		priv->can.can_stats.bus_error++;
> +		ndev->stats.rx_errors += rx_errors;
> +		ndev->stats.tx_errors += tx_errors;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BEIF);
> +	}
> +	if (eifr & EIFR_EWIF) {
> +		netdev_dbg(priv->ndev, "Error warning interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_WARNING;
> +		priv->can.can_stats.error_warning++;
> +		cf->data[1] |= txerr > rxerr ? CAN_ERR_CRTL_TX_WARNING :
> +					       CAN_ERR_CRTL_RX_WARNING;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);
> +	}
> +	if (eifr & EIFR_EPIF) {
> +		netdev_dbg(priv->ndev, "Error passive interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +		priv->can.can_stats.error_passive++;
> +		cf->data[1] |= txerr > rxerr ? CAN_ERR_CRTL_TX_PASSIVE :
> +					       CAN_ERR_CRTL_RX_PASSIVE;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
> +	}
> +	if (eifr & EIFR_BOEIF) {
> +		netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
> +		tx_failure_cleanup(ndev);
> +		priv->ier = IER_ERSIE;
> +		rcar_can_writeb(priv, RCAR_CAN_IER, priv->ier);
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		/* Clear interrupt condition */
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
> +		can_bus_off(ndev);
> +	}

You should not call this function in case of automatic recovery. Anyway,
as suggested above there is no need to support automatic recovery.

> +	if (eifr & EIFR_BORIF) {
> +		netdev_dbg(priv->ndev, "Bus-off recovery interrupt\n");
> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +		cf->can_id |= CAN_ERR_RESTARTED;

Does this interrupt come when Bus-off recovery has compeleted (back to
error-active?) CAN_ERR_RESTARTED should be reported when the bus-off
recovery has been triggered. It takes some time before it is really
completed.

> +		rcar_can_start(priv->ndev);
> +		priv->can.can_stats.restarts++;
> +		netif_carrier_on(ndev);
> +		netif_wake_queue(ndev);
> +	}
> +	if (eifr & EIFR_ORIF) {
> +		netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
> +		cf->can_id |= CAN_ERR_CRTL;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_ORIF);
> +	}
> +	if (eifr & EIFR_OLIF) {
> +		netdev_dbg(priv->ndev,
> +			   "Overload Frame Transmission error interrupt\n");
> +		cf->can_id |= CAN_ERR_PROT;
> +		cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> +		ndev->stats.rx_over_errors++;
> +		ndev->stats.rx_errors++;
> +		rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_OLIF);

No unnecessary casts please.

> +	}
> +
> +	netif_rx(skb);
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = (struct net_device *)dev_id;
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	u8 isr;
> +
> +	isr = rcar_can_readb(priv, RCAR_CAN_ISR);
> +	if (!(isr & priv->ier))
> +		return IRQ_NONE;
> +
> +	if (isr & ISR_ERSF)
> +		rcar_can_error(ndev);
> +
> +	if (isr & ISR_TXMF) {
> +		u32 ie_mask = 0;
> +		int i;
> +
> +		/* Set Transmit Mailbox Search Mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);
> +		for (i = 0; i < N_TX_MB; i++) {
> +			u8 mctl, mbx;
> +
> +			mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +			if (mbx & MSSR_SEST)
> +				break;
> +			mbx &= MSSR_MBNST;
> +			stats->tx_bytes += readb(&priv->regs->mb[mbx].dlc);
> +			stats->tx_packets++;
> +			mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +			/* Bits SENTDATA and TRMREQ cannot be
> +			 * set to 0 simultaneously
> +			 */
> +			mctl &= ~MCTL_TRMREQ;
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> +			mctl &= ~MCTL_SENTDATA;
> +			/* Clear interrupt */
> +			rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> +			ie_mask |= BIT(mbx - FIRST_TX_MB);
> +			can_get_echo_skb(ndev, mbx - FIRST_TX_MB);
> +			can_led_event(ndev, CAN_LED_EVENT_TX);
> +		}
> +		/* Set receive mailbox search mode */
> +		rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_RXMB);
> +		/* Disable mailbox interrupt, mark mailbox as free */
> +		if (ie_mask) {
> +			u32 mier1;
> +
> +			spin_lock(&priv->mier_lock);
> +			mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +			rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
> +			spin_unlock(&priv->mier_lock);
> +			netif_wake_queue(ndev);
> +		}
> +	}

Would be nice to have this in an (inline) function, like for the error
handling above.

> +	if (isr & ISR_RXM1F) {
> +		if (napi_schedule_prep(&priv->napi)) {
> +			/* Disable Rx interrupts */
> +			priv->ier &= ~IER_RXM1IE;
> +			rcar_can_writeb(priv, RCAR_CAN_IER, priv->ier);
> +			__napi_schedule(&priv->napi);
> +		}
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static int rcar_can_set_bittiming(struct net_device *dev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	u32 bcr;
> +	u16 ctlr;
> +	u8 clkr;
> +
> +	clk_prepare_enable(priv->clk);
> +	/* rcar_can_set_bittiming() is called in CAN sleep mode.
> +	 * Can write to BCR  in CAN reset mode or CAN halt mode.
> +	 * Cannot write to CLKR in halt mode, so go to reset mode.
> +	 */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	ctlr |= CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	/* Don't overwrite CLKR with 32-bit BCR access */
> +	/* CLKR has 8-bit access */
> +	clkr = rcar_can_readb(priv, RCAR_CAN_CLKR);
> +	bcr = BCR_TSEG1(bt->phase_seg1 + bt->prop_seg - 1) |
> +	      BCR_BPR(bt->brp - 1) | BCR_SJW(bt->sjw - 1) |
> +	      BCR_TSEG2(bt->phase_seg2 - 1);
> +	rcar_can_writel(priv, RCAR_CAN_BCR, bcr);
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, clkr);
> +	ctlr |= CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	clk_disable_unprepare(priv->clk);
> +	return 0;
> +}
> +
> +static void rcar_can_start(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr, n;
> +
> +	/* Set controller to known mode:
> +	 * - normal mailbox mode (no FIFO);
> +	 * - accept all messages (no filter).
> +	 * CAN is in sleep mode after MCU hardware or software reset.
> +	 */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	/* Go to reset mode */
> +	ctlr |= CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr |= CTLR_IDFM_MIXED; /* Select mixed ID mode */
> +	ctlr |= CTLR_TPM;	/* Set mailbox number priority transmit mode */
> +	ctlr &= ~CTLR_BOM;	/* Automatic recovery */
> +				/* compliant with ISO11898-1 */
> +	if (!autorecovery || priv->can.restart_ms)
> +		ctlr |= CTLR_BOM_ENT;	/* Entry to halt mode automatically */
> +					/* at bus-off */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +
> +	rcar_can_writeb(priv, RCAR_CAN_CLKR, priv->clock_select);
> +
> +	/* Accept all SID and EID */
> +	for (n = 0; n < N_RX_MKREGS; n++)
> +		rcar_can_writel(priv, RCAR_CAN_MKR(n), 0);
> +	rcar_can_writel(priv, RCAR_CAN_MKIVLR0, 0);
> +
> +	/* Initial value of MIER1 undefined.  Mark all Tx mailboxes as free. */
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +
> +	priv->ier = IER_TXMIE | IER_ERSIE | IER_RXM1IE;
> +	rcar_can_writeb(priv, RCAR_CAN_IER, priv->ier);
> +
> +	/* Accumulate error codes */
> +	rcar_can_writeb(priv, RCAR_CAN_ECSR, ECSR_EDPM);
> +	/* Enable error interrupts */
> +	rcar_can_writeb(priv, RCAR_CAN_EIER,
> +			EIER_EWIE | EIER_EPIE | EIER_BOEIE |
> +			(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING ?
> +			EIER_BEIE : 0) | EIER_ORIE | EIER_OLIE | EIER_BORIE);
> +	/* Enable interrupts for RX mailboxes */
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, RX_MBX_MASK);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	/* Write to the CiMCTLj register in CAN
> +	 * operation mode or CAN halt mode.
> +	 * Configure mailboxes 0-31 as Rx mailboxes.
> +	 * Configure mailboxes 32-63 as Tx mailboxes.
> +	 */
> +	/* Go to halt mode */
> +	ctlr |= CTLR_HALT;
> +	ctlr &= ~CTLR_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	for (n = 0; n < FIRST_TX_MB; n++) {
> +		/* According to documentation we should clear MCTL
> +		 * register before configuring mailbox.
> +		 */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), 0);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(n), MCTL_RECREQ);
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(FIRST_TX_MB + n), 0);
> +	}
> +	/* Go to operation mode */
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr & ~CTLR_FORCE_RESET);
> +}
> +
> +static int rcar_can_open(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	int err;
> +
> +	clk_prepare_enable(priv->clk);
> +	err = open_candev(ndev);
> +	if (err) {
> +		netdev_err(ndev, "open_candev() failed %d\n", err);
> +		goto out;
> +	}
> +	napi_enable(&priv->napi);
> +	err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
> +	if (err) {
> +		netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
> +		goto out_close;
> +	}
> +	can_led_event(ndev, CAN_LED_EVENT_OPEN);
> +	rcar_can_start(ndev);
> +	netif_start_queue(ndev);
> +	return 0;
> +out_close:
> +	napi_disable(&priv->napi);
> +	close_candev(ndev);
> +	clk_disable_unprepare(priv->clk);
> +out:
> +	return err;
> +}
> +
> +static void rcar_can_stop(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	/* Go to (force) reset mode */
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr |=  CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	rcar_can_writel(priv, RCAR_CAN_MIER0, 0);
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> +	rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> +	/* Go to sleep mode */
> +	ctlr |= CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +static int rcar_can_close(struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> +	netif_stop_queue(ndev);
> +	rcar_can_stop(ndev);
> +	free_irq(ndev->irq, ndev);
> +	napi_disable(&priv->napi);
> +	clk_disable_unprepare(priv->clk);
> +	close_candev(ndev);
> +	can_led_event(ndev, CAN_LED_EVENT_STOP);
> +	return 0;
> +}
> +
> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
> +				       struct net_device *ndev)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	u32 data, mier1, mbxno, i;
> +	unsigned long flags;
> +	u8 mctl = 0;
> +
> +	if (can_dropped_invalid_skb(ndev, skb))
> +		return NETDEV_TX_OK;
> +
> +	spin_lock_irqsave(&priv->mier_lock, flags);
> +	mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> +	if (mier1) {
> +		i = __builtin_clz(mier1);
> +		mbxno = i ? N_MBX - i : FIRST_TX_MB;
> +	} else {
> +		mbxno = FIRST_TX_MB;
> +	}
> +	mier1 |= BIT(mbxno - FIRST_TX_MB);
> +	rcar_can_writel(priv, RCAR_CAN_MIER1, mier1);
> +	spin_unlock_irqrestore(&priv->mier_lock, flags);
> +	if (unlikely(mier1 == 0xffffffff))
> +		netif_stop_queue(ndev);
> +
> +	if (cf->can_id & CAN_EFF_FLAG) {
> +		/* Extended frame format */
> +		data = (cf->can_id & CAN_EFF_MASK) | RCAR_CAN_IDE;
> +	} else {
> +		/* Standard frame format */
> +		data = (cf->can_id & CAN_SFF_MASK) << RCAR_CAN_SID_SHIFT;
> +	}
> +	if (cf->can_id & CAN_RTR_FLAG) {
> +		/* Remote transmission request */
> +		data |= RCAR_CAN_RTR;
> +	}
> +	writel(data, &priv->regs->mb[mbxno].id);
> +
> +	writeb(cf->can_dlc, &priv->regs->mb[mbxno].dlc);
> +
> +	for (i = 0; i < cf->can_dlc; i++)
> +		writeb(cf->data[i], &priv->regs->mb[mbxno].data[i]);
> +
> +	can_put_echo_skb(skb, ndev, mbxno - FIRST_TX_MB);
> +
> +	priv->ier |= IER_TXMIE;
> +	rcar_can_writeb(priv, RCAR_CAN_IER, priv->ier);
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> +		mctl |= MCTL_ONESHOT;
> +
> +	/* Start TX */
> +	mctl |= MCTL_TRMREQ;
> +	rcar_can_writeb(priv, RCAR_CAN_MCTL(mbxno), mctl);
> +	return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops rcar_can_netdev_ops = {
> +	.ndo_open = rcar_can_open,
> +	.ndo_stop = rcar_can_close,
> +	.ndo_start_xmit = rcar_can_start_xmit,
> +};
> +
> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
> +{
> +	struct net_device_stats *stats = &priv->ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u32 data;
> +	u8 dlc;
> +
> +	skb = alloc_can_skb(priv->ndev, &cf);
> +	if (!skb) {
> +		stats->rx_dropped++;
> +		return;
> +	}
> +
> +	data = readl(&priv->regs->mb[mbx].id);
> +	if (data & RCAR_CAN_IDE)
> +		cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
> +	else
> +		cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
> +
> +	dlc = readb(&priv->regs->mb[mbx].dlc);
> +	cf->can_dlc = get_can_dlc(dlc);
> +	if (data & RCAR_CAN_RTR) {
> +		cf->can_id |= CAN_RTR_FLAG;
> +	} else {
> +		for (dlc = 0; dlc < cf->can_dlc; dlc++)
> +			cf->data[dlc] = readb(&priv->regs->mb[mbx].data[dlc]);
> +	}
> +
> +	can_led_event(priv->ndev, CAN_LED_EVENT_RX);
> +
> +	netif_receive_skb(skb);
> +	stats->rx_bytes += cf->can_dlc;
> +	stats->rx_packets++;
> +}
> +
> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> +	struct rcar_can_priv *priv = container_of(napi,
> +						  struct rcar_can_priv, napi);
> +	int num_pkts = 0;
> +
> +	/* Find mailbox */
> +	while (num_pkts < quota) {
> +		u8 mctl, mbx;
> +
> +		mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> +		if (mbx & MSSR_SEST)
> +			break;
> +		mbx &= MSSR_MBNST;
> +		mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> +		/* Clear interrupt */
> +		rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
> +				mctl & ~MCTL_NEWDATA);
> +		rcar_can_rx_pkt(priv, mbx);
> +		++num_pkts;
> +	}
> +	/* All packets processed */
> +	if (num_pkts < quota) {
> +		napi_complete(napi);
> +		priv->ier |= IER_RXM1IE;
> +		rcar_can_writeb(priv, RCAR_CAN_IER, priv->ier);
> +	}
> +	return num_pkts;
> +}
> +
> +static int rcar_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		rcar_can_start(ndev);
> +		netif_wake_queue(ndev);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int rcar_can_get_berr_counter(const struct net_device *dev,
> +				     struct can_berr_counter *bec)
> +{
> +	struct rcar_can_priv *priv = netdev_priv(dev);
> +
> +	clk_prepare_enable(priv->clk);
> +	bec->txerr = rcar_can_readb(priv, RCAR_CAN_TECR);
> +	bec->rxerr = rcar_can_readb(priv, RCAR_CAN_RECR);
> +	clk_disable_unprepare(priv->clk);
> +	return 0;
> +}
> +
> +static int rcar_can_probe(struct platform_device *pdev)
> +{
> +	struct rcar_can_platform_data *pdata;
> +	struct rcar_can_priv *priv;
> +	struct net_device *ndev;
> +	struct resource *mem;
> +	void __iomem *addr;
> +	int err = -ENODEV;
> +	int irq;
> +
> +	pdata = dev_get_platdata(&pdev->dev);
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "No platform data provided!\n");
> +		goto fail;
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (!irq) {
> +		dev_err(&pdev->dev, "No IRQ resource\n");
> +		goto fail;
> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	addr = devm_ioremap_resource(&pdev->dev, mem);
> +	if (IS_ERR(addr)) {
> +		err = PTR_ERR(addr);
> +		goto fail;
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct rcar_can_priv), N_MBX - FIRST_TX_MB);
> +	if (!ndev) {
> +		dev_err(&pdev->dev, "alloc_candev failed\n");
> +		err = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	priv = netdev_priv(ndev);
> +
> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		err = PTR_ERR(priv->clk);
> +		dev_err(&pdev->dev, "cannot get clock: %d\n", err);
> +		goto fail_clk;
> +	}
> +
> +	ndev->netdev_ops = &rcar_can_netdev_ops;
> +	ndev->irq = irq;
> +	ndev->flags |= IFF_ECHO;
> +	priv->ndev = ndev;
> +	priv->regs = (struct rcar_can_regs *)addr;

Is this cast needed?

> +	priv->clock_select = pdata->clock_select;
> +	priv->can.clock.freq = clk_get_rate(priv->clk);
> +	priv->can.bittiming_const = &rcar_can_bittiming_const;
> +	priv->can.do_set_bittiming = rcar_can_set_bittiming;
> +	priv->can.do_set_mode = rcar_can_do_set_mode;
> +	priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING |
> +				       CAN_CTRLMODE_ONE_SHOT;
> +	platform_set_drvdata(pdev, ndev);
> +	SET_NETDEV_DEV(ndev, &pdev->dev);
> +	spin_lock_init(&priv->mier_lock);
> +
> +	netif_napi_add(ndev, &priv->napi, rcar_can_rx_poll,
> +		       RCAR_CAN_NAPI_WEIGHT);
> +	err = register_candev(ndev);
> +	if (err) {
> +		dev_err(&pdev->dev, "register_candev() failed\n");
> +		goto fail_candev;
> +	}
> +
> +	devm_can_led_init(ndev);
> +
> +	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
> +		 priv->regs, ndev->irq);
> +
> +	return 0;
> +fail_candev:
> +	netif_napi_del(&priv->napi);
> +fail_clk:
> +	free_candev(ndev);
> +fail:
> +	return err;
> +}
> +
> +static int rcar_can_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> +	unregister_candev(ndev);
> +	netif_napi_del(&priv->napi);
> +	free_candev(ndev);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rcar_can_suspend(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	if (netif_running(ndev)) {
> +		netif_stop_queue(ndev);
> +		netif_device_detach(ndev);
> +	}
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr |= CTLR_HALT;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr |= CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	priv->can.state = CAN_STATE_SLEEPING;
> +
> +	clk_disable(priv->clk);
> +	return 0;
> +}
> +
> +static int rcar_can_resume(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct rcar_can_priv *priv = netdev_priv(ndev);
> +	u16 ctlr;
> +
> +	clk_enable(priv->clk);
> +
> +	ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> +	ctlr &= ~CTLR_SLPM;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	ctlr &= ~CTLR_FORCE_RESET;
> +	rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	if (netif_running(ndev)) {
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rcar_can_pm_ops, rcar_can_suspend, rcar_can_resume);
> +
> +static struct platform_driver rcar_can_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.pm = &rcar_can_pm_ops,
> +	},
> +	.probe = rcar_can_probe,
> +	.remove = rcar_can_remove,
> +};
> +
> +module_platform_driver(rcar_can_driver);
> +
> +MODULE_AUTHOR("Cogent Embedded, Inc.");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN driver for Renesas R-Car SoC");
> +MODULE_ALIAS("platform:" DRV_NAME);
> Index: linux-can-next/include/linux/can/platform/rcar_can.h
> ===================================================================
> --- /dev/null
> +++ linux-can-next/include/linux/can/platform/rcar_can.h
> @@ -0,0 +1,15 @@
> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
> +#define _CAN_PLATFORM_RCAR_CAN_H_
> +
> +#include <linux/types.h>
> +
> +/* Clock Select Register settings */
> +#define CLKR_CLKEXT	3	/* Externally input clock */
> +#define CLKR_CLKP2	1	/* Peripheral clock (clkp2) */
> +#define CLKR_CLKP1	0	/* Peripheral clock (clkp1) */
> +
> +struct rcar_can_platform_data {
> +	u8 clock_select;	/* Clock source select */
> +};
> +
> +#endif	/* !_CAN_PLATFORM_RCAR_CAN_H_ */
> --

Wolfgang.


^ permalink raw reply

* [PATCH net-next] ipv4: fix DO and PROBE pmtu mode regarding local fragmentation with UFO/CORK
From: Hannes Frederic Sowa @ 2013-10-25 19:40 UTC (permalink / raw)
  To: netdev

UFO as well as UDP_CORK do not respect IP_PMTUDISC_DO and
IP_PMTUDISC_PROBE well enough.

UFO enabled packet delivery just appends all frags to the cork and hands
it over to the network card. So we just deliver non-DF udp fragments
(DF-flag may get overwritten by hardware or virtual UFO enabled
interface).

UDP_CORK does enqueue the data until the cork is disengaged. At this
point it sets the correct IP_DF and local_df flags and hands it over to
ip_fragment which in this case will generate an icmp error which gets
appended to the error socket queue. This is not reflected in the syscall
error (of course, if UFO is enabled this also won't happen).

Improve this by checking the pmtudisc flags before appending data to the
socket and if we still can fit all data in one packet when IP_PMTUDISC_DO
or IP_PMTUDISC_PROBE is set, only then proceed.

Maybe, we can relax some other checks around ip_fragment. This needs
more research.

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv4/ip_output.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 8fbac7d..27dc1b3 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -810,7 +810,7 @@ static int __ip_append_data(struct sock *sk,
 	int copy;
 	int err;
 	int offset = 0;
-	unsigned int maxfraglen, fragheaderlen;
+	unsigned int maxfraglen, fragheaderlen, maxmsgsize;
 	int csummode = CHECKSUM_NONE;
 	struct rtable *rt = (struct rtable *)cork->dst;
 
@@ -823,8 +823,10 @@ static int __ip_append_data(struct sock *sk,
 
 	fragheaderlen = sizeof(struct iphdr) + (opt ? opt->optlen : 0);
 	maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen;
+	maxmsgsize = (inet->pmtudisc >= IP_PMTUDISC_DO) ?
+		     maxfraglen : 0xFFFF;
 
-	if (cork->length + length > 0xFFFF - fragheaderlen) {
+	if (cork->length + length > maxmsgsize - fragheaderlen) {
 		ip_local_error(sk, EMSGSIZE, fl4->daddr, inet->inet_dport,
 			       mtu-exthdrlen);
 		return -EMSGSIZE;
@@ -1122,7 +1124,7 @@ ssize_t	ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
 	int mtu;
 	int len;
 	int err;
-	unsigned int maxfraglen, fragheaderlen, fraggap;
+	unsigned int maxfraglen, fragheaderlen, fraggap, maxmsgsize;
 
 	if (inet->hdrincl)
 		return -EPERM;
@@ -1146,8 +1148,10 @@ ssize_t	ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
 
 	fragheaderlen = sizeof(struct iphdr) + (opt ? opt->optlen : 0);
 	maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen;
+	maxmsgsize = (inet->pmtudisc >= IP_PMTUDISC_DO) ?
+		     maxfraglen : 0xFFFF;
 
-	if (cork->length + size > 0xFFFF - fragheaderlen) {
+	if (cork->length + size > maxmsgsize - fragheaderlen) {
 		ip_local_error(sk, EMSGSIZE, fl4->daddr, inet->inet_dport, mtu);
 		return -EMSGSIZE;
 	}
-- 
1.8.3.1

^ permalink raw reply related

* Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: Hannes Frederic Sowa @ 2013-10-25 20:12 UTC (permalink / raw)
  To: Vladislav Yasevich
  Cc: Jiri Pirko, netdev@vger.kernel.org, David Miller, kuznet, jmorris,
	yoshfuji, kaber, thaller, Stephen Hemminger
In-Reply-To: <CAGCdqXG6pOdaHNDsKnMGWKRs8Me7F8oyWVcgmtGP8ng2XRGfcQ@mail.gmail.com>

On Fri, Oct 25, 2013 at 06:05:40AM -0400, Vladislav Yasevich wrote:
> On Thu, Oct 24, 2013 at 12:59 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> > Thu, Oct 24, 2013 at 04:02:53PM CEST, hannes@stressinduktion.org wrote:
> >>On Thu, Oct 24, 2013 at 03:45:55PM +0200, Jiri Pirko wrote:
> >>> This is needed in order to implement userspace address configuration,
> >>> namely ip6-privacy (rfc4941) in NetworkManager.
> >>>
> >>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >>> ---
> >>>  net/ipv6/addrconf.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> >>> index cd3fb30..962c7c9 100644
> >>> --- a/net/ipv6/addrconf.c
> >>> +++ b/net/ipv6/addrconf.c
> >>> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
> >>>              return -ENODEV;
> >>>
> >>>      /* We ignore other flags so far. */
> >>> -    ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
> >>> +    ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
> >>> +                                  IFA_F_TEMPORARY);
> >>>
> >>>      ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
> >>>      if (ifa == NULL) {
> >>
> >>Hm, the kernel will pick up IFA_F_TEMPORARY marked addresses and do ipv6 address
> >>regeneration (depending on lifetimes). Is this intended?
> >
> > I think that that behaviour is valid. It is in compliance with valid lft and
> > preferred lft behaviour.
> >
> 
> Actually, I don't think it would do the regeneration since ifpub
> pointer is not set.
> 
> I appears that the temp address management would have to be done in
> userspace.

Sorry, yes this is correct. I should have looked at the source and not answer
just from memory.

Thanks,

  Hannes

^ permalink raw reply

* Re: [gpio:for-next 67/67] pch_gbe_main.c:undefined reference to `devm_gpio_request_one'
From: David Cohen @ 2013-10-25 21:21 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Darren Hart, David S. Miller, netdev@vger.kernel.org,
	Fengguang Wu, Alexandre Courbot, linux-gpio@vger.kernel.org
In-Reply-To: <CACRpkdYDUKK-jAeeHECfRcKM+-MDuyarDMGDP0p39U_-WLgd8w@mail.gmail.com>

Hi Linus,

On 10/25/2013 03:49 AM, Linus Walleij wrote:
> On Fri, Oct 25, 2013 at 12:41 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
>
>>> I wouldn't object to adding a dependency to GPIO_PCH and GPIOLIB
>>> unconditionally for PCH_GBE as GPIO_PCH is the same chip... but I don't
>>> know if David Miller would be amenable to that.
>>
>> Well we should probably just stick a dependency to GPIOLIB in there.
>>
>> - It #includes <linux/gpio.h>
>> - It uses gpiolib functions to do something vital
>>
>> It was just happy that dummy versions were slotted in until now.
>
> ...or maybe I'm just confused now?
>
> Should we just add a static inline stub of devm_gpio_request_one()?

I am not familiar with the HW. But checking the code, platform 
initialization should fail with a dummy devm_gpio_request_one()
implementation. IMO it makes more sense to depend on GPIOLIB.

Br, David Cohen

^ permalink raw reply

* Re: [gpio:for-next 67/67] pch_gbe_main.c:undefined reference to `devm_gpio_request_one'
From: David Cohen @ 2013-10-25 21:25 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Darren Hart, David S. Miller, netdev@vger.kernel.org,
	Fengguang Wu, Alexandre Courbot, linux-gpio@vger.kernel.org
In-Reply-To: <526AE0EB.4020602@linux.intel.com>

On 10/25/2013 02:21 PM, David Cohen wrote:
> Hi Linus,
>
> On 10/25/2013 03:49 AM, Linus Walleij wrote:
>> On Fri, Oct 25, 2013 at 12:41 PM, Linus Walleij
>> <linus.walleij@linaro.org> wrote:
>>
>>>> I wouldn't object to adding a dependency to GPIO_PCH and GPIOLIB
>>>> unconditionally for PCH_GBE as GPIO_PCH is the same chip... but I don't
>>>> know if David Miller would be amenable to that.
>>>
>>> Well we should probably just stick a dependency to GPIOLIB in there.
>>>
>>> - It #includes <linux/gpio.h>
>>> - It uses gpiolib functions to do something vital
>>>
>>> It was just happy that dummy versions were slotted in until now.
>>
>> ...or maybe I'm just confused now?
>>
>> Should we just add a static inline stub of devm_gpio_request_one()?
>
> I am not familiar with the HW. But checking the code, platform
> initialization should fail with a dummy devm_gpio_request_one()
> implementation. IMO it makes more sense to depend on GPIOLIB.

Actually, forget about it. Despite driver_data->platform_init() may
return error, probe() never checks for it. I think the driver must be
fixed, but in meanwhile a static inline stub seems reasonable.

>
> Br, David Cohen


^ permalink raw reply

* [PATCH net-next] macsonic: Updated printk() statement to netdev_info function
From: Matt Zanchelli @ 2013-10-25 21:53 UTC (permalink / raw)
  To: netdev; +Cc: Matt Zanchelli

Updated the printk() statement in mac_sonic_probe() to recommended netdev_info.

Reviewed-by: Mukkai Krishnamoorthy <mskmoorthy@gmail.com>
Reviewed-by: Maxwell Ensley-Field <mensleyfield@gmail.com>
Reviewed-by: Nicole Negedly <nnegedly@gmail.com>
Reviewed-by: Daniel Felizardo <danfelizardo@gmail.com>
Signed-off-by: Matt Zanchelli <zanchm@rpi.edu>
---
 drivers/net/ethernet/natsemi/macsonic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index 346a4e0..9e9b3b0 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -601,7 +601,7 @@ found:
 	if (err)
 		goto out;
 
-	printk("%s: MAC %pM IRQ %d\n", dev->name, dev->dev_addr, dev->irq);
+	netdev_info(dev, "MAC %pM IRQ %d\n", dev->dev_addr, dev->irq);
 
 	return 0;
 
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH net-next] WD80x3: fix printk() calls to netdev_*() calls
From: Drew McGowen @ 2013-10-25 21:56 UTC (permalink / raw)
  To: netdev; +Cc: Drew McGowen

Replace legacy-style printk() calls to use netdev_<level> functions. Also fix
version printing to use pr_info_once().

Reviewed-by: Ben Pringle <ben.pringle@gmail.com>
Signed-off-by: Drew McGowen <quantumdude836@gmail.com>
---
 drivers/net/ethernet/8390/wd.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/8390/wd.c b/drivers/net/ethernet/8390/wd.c
index 03eb3ee..38d3110 100644
--- a/drivers/net/ethernet/8390/wd.c
+++ b/drivers/net/ethernet/8390/wd.c
@@ -26,7 +26,7 @@
 */
 
 static const char version[] =
-	"wd.c:v1.10 9/23/94 Donald Becker (becker@cesdis.gsfc.nasa.gov)\n";
+	"wd.c:v1.10 9/23/94 Donald Becker (becker@cesdis.gsfc.nasa.gov)";
 
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -169,7 +169,6 @@ static int __init wd_probe1(struct net_device *dev, int ioaddr)
 	int ancient = 0;			/* An old card without config registers. */
 	int word16 = 0;				/* 0 = 8 bit, 1 = 16 bit */
 	const char *model_name;
-	static unsigned version_printed;
 
 	for (i = 0; i < 8; i++)
 		checksum += inb(ioaddr + 8 + i);
@@ -180,19 +179,19 @@ static int __init wd_probe1(struct net_device *dev, int ioaddr)
 
 	/* Check for semi-valid mem_start/end values if supplied. */
 	if ((dev->mem_start % 0x2000) || (dev->mem_end % 0x2000)) {
-		printk(KERN_WARNING "wd.c: user supplied mem_start or mem_end not on 8kB boundary - ignored.\n");
+		netdev_warn(dev, "wd.c: user supplied mem_start or mem_end not on 8kB boundary - ignored.\n");
 		dev->mem_start = 0;
 		dev->mem_end = 0;
 	}
 
-	if (ei_debug  &&  version_printed++ == 0)
-		printk(version);
+	if (ei_debug)
+		pr_info_once("%s\n", version);
 
 	for (i = 0; i < 6; i++)
 		dev->dev_addr[i] = inb(ioaddr + 8 + i);
 
-	printk("%s: WD80x3 at %#3x, %pM",
-	       dev->name, ioaddr, dev->dev_addr);
+	netdev_info(dev, "WD80x3 at %#3x, %pM",
+		ioaddr, dev->dev_addr);
 
 	/* The following PureData probe code was contributed by
 	   Mike Jagdis <jaggy@purplet.demon.co.uk>. Puredata does software
@@ -244,7 +243,7 @@ static int __init wd_probe1(struct net_device *dev, int ioaddr)
 		}
 #ifndef final_version
 		if ( !ancient && (inb(ioaddr+1) & 0x01) != (word16 & 0x01))
-			printk("\nWD80?3: Bus width conflict, %d (probe) != %d (reg report).",
+			netdev_warn(dev, "\nWD80?3: Bus width conflict, %d (probe) != %d (reg report).",
 				   word16 ? 16 : 8, (inb(ioaddr+1) & 0x01) ? 16 : 8);
 #endif
 	}
@@ -259,7 +258,7 @@ static int __init wd_probe1(struct net_device *dev, int ioaddr)
 		if (reg0 == 0xff || reg0 == 0) {
 			/* Future plan: this could check a few likely locations first. */
 			dev->mem_start = 0xd0000;
-			printk(" assigning address %#lx", dev->mem_start);
+			pr_cont(" assigning address %#lx", dev->mem_start);
 		} else {
 			int high_addr_bits = inb(ioaddr+WD_CMDREG5) & 0x1f;
 			/* Some boards don't have the register 5 -- it returns 0xff. */
@@ -298,7 +297,7 @@ static int __init wd_probe1(struct net_device *dev, int ioaddr)
 			outb_p(0x00, nic_addr+EN0_IMR);	/* Mask all intrs. again. */
 
 			if (ei_debug > 2)
-				printk(" autoirq is %d", dev->irq);
+				pr_cont(" autoirq is %d", dev->irq);
 			if (dev->irq < 2)
 				dev->irq = word16 ? 10 : 5;
 		} else
@@ -310,7 +309,7 @@ static int __init wd_probe1(struct net_device *dev, int ioaddr)
 	   share and the board will usually be enabled. */
 	i = request_irq(dev->irq, ei_interrupt, 0, DRV_NAME, dev);
 	if (i) {
-		printk (" unable to get IRQ %d.\n", dev->irq);
+		pr_cont(" unable to get IRQ %d.\n", dev->irq);
 		return i;
 	}
 
@@ -338,7 +337,7 @@ static int __init wd_probe1(struct net_device *dev, int ioaddr)
 		return -ENOMEM;
 	}
 
-	printk(" %s, IRQ %d, shared memory at %#lx-%#lx.\n",
+	pr_cont(" %s, IRQ %d, shared memory at %#lx-%#lx.\n",
 		   model_name, dev->irq, dev->mem_start, dev->mem_end-1);
 
 	ei_status.reset_8390 = wd_reset_8390;
@@ -387,7 +386,8 @@ wd_reset_8390(struct net_device *dev)
 	int wd_cmd_port = dev->base_addr - WD_NIC_OFFSET; /* WD_CMDREG */
 
 	outb(WD_RESET, wd_cmd_port);
-	if (ei_debug > 1) printk("resetting the WD80x3 t=%lu...", jiffies);
+	if (ei_debug > 1)
+		netdev_dbg(dev, "resetting the WD80x3 t=%lu...", jiffies);
 	ei_status.txing = 0;
 
 	/* Set up the ASIC registers, just in case something changed them. */
@@ -395,7 +395,8 @@ wd_reset_8390(struct net_device *dev)
 	if (ei_status.word16)
 		outb(NIC16 | ((dev->mem_start>>19) & 0x1f), wd_cmd_port+WD_CMDREG5);
 
-	if (ei_debug > 1) printk("reset done\n");
+	if (ei_debug > 1)
+		pr_cont("reset done\n");
 }
 
 /* Grab the 8390 specific header. Similar to the block_input routine, but
@@ -476,7 +477,7 @@ wd_close(struct net_device *dev)
 	int wd_cmdreg = dev->base_addr - WD_NIC_OFFSET; /* WD_CMDREG */
 
 	if (ei_debug > 1)
-		printk("%s: Shutting down ethercard.\n", dev->name);
+		netdev_info(dev, "Shutting down ethercard.\n");
 	ei_close(dev);
 
 	/* Change from 16-bit to 8-bit shared memory so reboot works. */
-- 
1.8.1.2

^ permalink raw reply related

* Re: crypto: skcipher - Use eseqiv even on UP machines
From: David Miller @ 2013-10-25 21:57 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev, linux-crypto
In-Reply-To: <20131025065049.GB31491@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 25 Oct 2013 08:50:49 +0200

> On Thu, Oct 24, 2013 at 08:41:49PM +0800, Herbert Xu wrote:
>> Hi:
>>     
>> Previously we would use eseqiv on all async ciphers in all cases,
>> and sync ciphers if we have more than one CPU.  This meant that
>> chainiv is only used in the case of sync ciphers on a UP machine.
>> 
>> As chainiv may aid attackers by making the IV predictable, even
>> though this risk itself is small, the above usage pattern causes
>> it to further leak information about the host.
>> 
>> This patch addresses these issues by using eseqiv even if we're
>> on a UP machine.
>> 
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>> 
> 
> That's fine by me.
> 
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

I'm ok with this too:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* [PATCH net-next] 3c515: Fix warning when building
From: Colin Rice @ 2013-10-25 21:59 UTC (permalink / raw)
  To: netdev; +Cc: Colin Rice


Signed-off-by: Colin Rice <ricec2@rpi.edu>
---
 drivers/net/ethernet/3com/3c515.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/3com/3c515.c b/drivers/net/ethernet/3com/3c515.c
index 94c656f..33ffda4 100644
--- a/drivers/net/ethernet/3com/3c515.c
+++ b/drivers/net/ethernet/3com/3c515.c
@@ -1063,7 +1063,7 @@ static netdev_tx_t corkscrew_start_xmit(struct sk_buff *skb,
 #ifdef VORTEX_BUS_MASTER
 	if (vp->bus_master) {
 		/* Set the bus-master controller to transfer the packet. */
-		outl((int) (skb->data), ioaddr + Wn7_MasterAddr);
+		outl((size_t) (skb->data), ioaddr + Wn7_MasterAddr);
 		outw((skb->len + 3) & ~3, ioaddr + Wn7_MasterLen);
 		vp->tx_skb = skb;
 		outw(StartDMADown, ioaddr + EL3_CMD);
-- 
1.8.1.2

^ permalink raw reply related

* Re: vxlan gso is broken by stackable gso_segment()
From: David Miller @ 2013-10-25 22:18 UTC (permalink / raw)
  To: eric.dumazet; +Cc: ast, edumazet, stephen, netdev
In-Reply-To: <1382692140.7572.79.camel@edumazet-glaptop.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 25 Oct 2013 02:09:00 -0700

> @@ -1252,6 +1252,7 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>  	const struct net_offload *ops;
>  	unsigned int offset = 0;
>  	struct iphdr *iph;
> +	bool udpfrag;
>  	bool tunnel;
>  	int proto;
>  	int nhoff;
> @@ -1306,10 +1307,11 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>  	if (IS_ERR_OR_NULL(segs))
>  		goto out;
>  
> +	udpfrag = !!skb->encapsulation && proto == IPPROTO_UDP;
>  	skb = segs;
>  	do {
>  		iph = (struct iphdr *)(skb_mac_header(skb) + nhoff);
> -		if (!tunnel && proto == IPPROTO_UDP) {
> +		if (udpfrag) {
>  			iph->id = htons(id);
>  			iph->frag_off = htons(offset >> 3);
>  			if (skb->next != NULL)
> 

The "tunnel" variable becomes unused once you do this, please remove it.

^ permalink raw reply

* Re: vxlan gso is broken by stackable gso_segment()
From: Alexei Starovoitov @ 2013-10-25 22:41 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, Eric Dumazet, stephen, netdev
In-Reply-To: <20131025.181809.348808013109842304.davem@davemloft.net>

On Fri, Oct 25, 2013 at 3:18 PM, David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 25 Oct 2013 02:09:00 -0700
>
>> @@ -1252,6 +1252,7 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>>       const struct net_offload *ops;
>>       unsigned int offset = 0;
>>       struct iphdr *iph;
>> +     bool udpfrag;
>>       bool tunnel;
>>       int proto;
>>       int nhoff;
>> @@ -1306,10 +1307,11 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>>       if (IS_ERR_OR_NULL(segs))
>>               goto out;
>>
>> +     udpfrag = !!skb->encapsulation && proto == IPPROTO_UDP;
>>       skb = segs;
>>       do {
>>               iph = (struct iphdr *)(skb_mac_header(skb) + nhoff);
>> -             if (!tunnel && proto == IPPROTO_UDP) {
>> +             if (udpfrag) {
>>                       iph->id = htons(id);
>>                       iph->frag_off = htons(offset >> 3);
>>                       if (skb->next != NULL)
>>
>
> The "tunnel" variable becomes unused once you do this, please remove it.

'bool tunnel' actually still used to indicate encap_level > 0

Eric's fix brings back performance for vxlan and gre keeps working. Thx!

net/core/skbuff.c:3474 skb_try_coalesce() warning, I mentioned before,
is unrelated.
I still see it with this patch. Running either gre or vxlan tunnels.

^ permalink raw reply

* Re: [PATCH RESEND] packet: Deliver VLAN TPID to userspace
From: David Miller @ 2013-10-25 22:59 UTC (permalink / raw)
  To: atzm; +Cc: netdev, stephen, bhutchings
In-Reply-To: <87r4bdk8c3.wl%atzm@stratosphere.co.jp>

From: Atzm Watanabe <atzm@stratosphere.co.jp>
Date: Tue, 22 Oct 2013 17:39:40 +0900

>  struct tpacket_hdr_variant1 {
>  	__u32	tp_rxhash;
>  	__u32	tp_vlan_tci;
> +	__u32	tp_vlan_tpid;
>  };
>  

You cannot do this, the header length is not variable.

This patch has been submitted several times, each of which you
have been shown ways in which your changes break userspace in
one way or another.

^ permalink raw reply

* Re: [PATCH net-next] netem: markov loss model transition fix
From: David Miller @ 2013-10-25 23:04 UTC (permalink / raw)
  To: hagen; +Cc: netdev, stephen, edumazet, stefano.salsano, fabio.ludovici
In-Reply-To: <1382477226-1869-1-git-send-email-hagen@jauu.net>

From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Tue, 22 Oct 2013 23:27:06 +0200

> The transition from markov state "3 => lost packets within a burst
> period" to "1 => successfully transmitted packets within a gap period"
> has no *additional* loss event. The loss already happen for transition
> from 1 -> 3, this additional loss will make things go wild.
> 
> E.g. transition probabilities:
> 
> p13:   10%
> p31:  100%
> 
> Expected:
> 
> Ploss = p13 / (p13 + p31)
> Ploss = ~9.09%
> 
> ... but it isn't. Even worse: we get a double loss - each time.
> So simple don't return true to indicate loss, rather break and return
> false.
> 
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Stefano Salsano <stefano.salsano@uniroma2.it>
> Cc: Fabio Ludovici <fabio.ludovici@yahoo.it>
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: make net_get_random_once irqsave
From: David Miller @ 2013-10-25 23:04 UTC (permalink / raw)
  To: hannes; +Cc: netdev, edumazet
In-Reply-To: <20131023111200.GB26236@order.stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Wed, 23 Oct 2013 13:12:00 +0200

> I initial build a non-irqsave version of net_get_random_once because I
> would liked to have the freedom to defer even the extraction process of
> get_random_bytes until the nonblocking pool is fully seeded.
> 
> I don't think this is a good idea anymore and thus this patch makes
> net_get_random_once irqsave. Now someone using net_get_random_once does
> not need to care from where it is called.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: initialize hashrnd in flow_dissector with net_get_random_once
From: David Miller @ 2013-10-25 23:04 UTC (permalink / raw)
  To: hannes; +Cc: netdev, edumazet
In-Reply-To: <20131023111219.GA31531@order.stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Wed, 23 Oct 2013 13:12:19 +0200

> We also can defer the initialization of hashrnd in flow_dissector
> to its first use. Since net_get_random_once is irqsave now we don't
> have to audit the call paths if one of this functions get called by an
> interrupt handler.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Applied.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox