On Wed, Nov 16, 2011 at 9:57 AM, Jesse Gross <je...@nicira.com> wrote:
> On Mon, Nov 14, 2011 at 2:01 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
>> Following patch deletes OVS compatibility code related to older kernel,
>> bridge, vlan etc and rearranges it for upstreaming.
>>
>> Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
>
> Can you update this for Ben's patches?
>
> I also got a bunch of whitespace errors when applying:
> Applying: net-ovs: Removal of kernel compatibility code from OVS
> /home/jesse/linux/.git/rebase-apply/patch:1661: space before tab in indent.
> * those we have collected (split into err_stats and percpu_stats) from
> /home/jesse/linux/.git/rebase-apply/patch:1662: space before tab in indent.
> * set_stats() and device error stats from netdev->get_stats() (for
> /home/jesse/linux/.git/rebase-apply/patch:1663: space before tab in indent.
> * errors that happen downstream and therefore aren't reported through
> /home/jesse/linux/.git/rebase-apply/patch:1664: space before tab in indent.
> * our vport_record_error() function).
>
>> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
>> index 49d93aa..b79128d 100644
>> --- a/net/openvswitch/datapath.c
>> +++ b/net/openvswitch/datapath.c
>> @@ -370,7 +256,6 @@ int dp_upcall(struct datapath *dp, struct sk_buff *skb,
>> goto err;
>>
>> return 0;
>> -
>> err:
>> stats = per_cpu_ptr(dp->stats_percpu, smp_processor_id());
>
> Extra whitespace change.
>
>> @@ -1772,24 +1602,15 @@ static int ovs_vport_cmd_new(struct sk_buff *skb,
>> struct genl_info *info)
>> if (IS_ERR(vport))
>> goto exit_unlock;
>>
>> - dp_sysfs_add_if(vport);
>> -
>> - err = change_vport(vport, a);
>> - if (!err) {
>> - reply = ovs_vport_cmd_build_info(vport, info->snd_pid,
>> - info->snd_seq,
>> - OVS_VPORT_CMD_NEW);
>> - if (IS_ERR(reply))
>> - err = PTR_ERR(reply);
>> - }
>> - if (err) {
>> + reply = ovs_vport_cmd_build_info(vport, info->snd_pid, info->snd_seq,
>> + OVS_VPORT_CMD_NEW);
>> + if (IS_ERR(reply)) {
>> + err = PTR_ERR(reply);
>> dp_detach_port(vport);
>> goto exit_unlock;
>> }
>> genl_notify(reply, genl_info_net(info), info->snd_pid,
>> dp_vport_multicast_group.id, info->nlhdr, GFP_KERNEL);
>> -
>> -
>> exit_unlock:
>> rtnl_unlock();
>
> Extra whitespace change.
>
>> @@ -2039,16 +1843,11 @@ static int __init dp_init(void)
>>
>> BUILD_BUG_ON(sizeof(struct ovs_skb_cb) > sizeof(dummy_skb->cb));
>>
>> - pr_info("Open vSwitch %s, built "__DATE__" "__TIME__"\n",
>> - VERSION BUILDNR);
>> -
>> - err = tnl_init();
>> - if (err)
>> - goto error;
>> + pr_info("Open vSwitch forwarding datapath\n");
>
> I realized that in the module info we use "Open vSwitch switching
> datapath" so we should probably be consistent. Maybe we should bring
> the string change over to the OVS repository as well.
>
>> diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
>> index c7014c3..86bb180 100644
>> --- a/net/openvswitch/datapath.h
>> +++ b/net/openvswitch/datapath.h
>
> Since you removed dp_ioctl_hook in other places we can remove the
> extern declaration from here as well.
>
>> diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
>> index 43360cc..4596fd2 100644
>> --- a/net/openvswitch/flow.h
>> +++ b/net/openvswitch/flow.h
>> @@ -44,7 +43,7 @@ struct sw_flow_key {
>> } eth;
>> struct {
>> u8 proto; /* IP protocol or lower 8 bits of ARP
>> opcode. */
>> - u8 tos; /* IP ToS. */
>> + u8 tos; /* IP ToS. */
>
> Whitespace change.
This is fixing white warning reported by checkpatch. So I will keep it.
I will send patch fixed according to other of comments.
>
>> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
>> index a9d9238..f37b34f 100644
>> --- a/net/openvswitch/vport-netdev.c
>> +++ b/net/openvswitch/vport-netdev.c
>> @@ -303,71 +158,15 @@ static int netdev_send(struct vport *vport, struct
>> sk_buff *skb)
> [...]
>> len = skb->len;
>> - dev_queue_xmit(skb);
>>
>> - return len;
>> + if (unlikely(dev_queue_xmit(skb)))
>> + goto error_record;
>>
>> + return len;
>> error:
>> kfree_skb(skb);
>> +error_record:
>> vport_record_error(vport, VPORT_E_TX_DROPPED);
>> return 0;
>
> This is a change in behavior that shouldn't go directly into the
> upstream version. It belongs in the OVS tree, probably as part of a
> larger stats cleanup. At this point, I'm not planning on pulling over
> any enhancements until this is accepted upstream, so let's wait on
> this.
>
>> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
>> index a6b686c..448de15 100644
>> --- a/net/openvswitch/vport.c
>> +++ b/net/openvswitch/vport.c
>> @@ -242,7 +218,6 @@ struct vport *vport_add(const struct vport_parms *parms)
>> }
>>
>> err = -EAFNOSUPPORT;
>> -
>> out:
>> return ERR_PTR(err);
>> }
>> @@ -278,56 +253,10 @@ void vport_del(struct vport *vport)
>> ASSERT_RTNL();
>>
>> hlist_del_rcu(&vport->hash_node);
>> -
>> vport->ops->destroy(vport);
>> }
>
> Two extra whitespace changes.
>
>> diff --git a/net/openvswitch/vport.h b/net/openvswitch/vport.h
>> index 3dbc68f..d494060 100644
>> --- a/net/openvswitch/vport.h
>> +++ b/net/openvswitch/vport.h
>> @@ -61,8 +59,6 @@ struct vport_err_stats {
>> * @rcu: RCU callback head for deferred destruction.
>> * @port_no: Index into @dp's @ports array.
>> * @dp: Datapath to which this port belongs.
>> - * @kobj: Represents /sys/class/net/<devname>/brport.
>> - * @linkname: The name of the link from /sys/class/net/<datapath>/brif to
>> this
>> * &struct vport. (We keep this around so that we can delete it if the
>> * device gets renamed.) Set to the null string when no link exists.
>
> You only deleted half the comment.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
