On Mon, Aug 21, 2017 at 05:14:46PM +0200, Amine Kherbouche wrote:
> > +#include <net/ip_tunnels.h>
>
> some headers are not needed.
Which ones? net/ip_tunnels.h is needed for ip_tunnel_get_stats64().
> > +static netdev_tx_t vpls_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > + int err = -EINVAL, ok_count = 0;
> > + struct vpls_priv *priv = netdev_priv(dev);
> > + struct vpls_info *vi;
> > + struct pcpu_sw_netstats *stats;
> > + size_t len = skb->len;
> > +
> > + rcu_read_lock();
> > + vi = skb_vpls_info(skb);
> > +
> > + skb_orphan(skb);
> > + skb_forward_csum(skb);
> > +
> > + if (vi) {
> > + err = vpls_xmit_wire(skb, dev, priv, vi->pw_label);
> > + if (err)
> > + goto out_err;
> > + } else {
>
> the rcu_read_lock() is just needed for the else statement right ?
Yeah, it's leftover from an earlier version where it was used for what
is now skb_vpls_info(). Fixed in next version.
> > +int vpls_rcv(struct sk_buff *skb, struct net_device *in_dev,
> > + struct packet_type *pt, struct mpls_route *rt,
> > + struct mpls_shim_hdr *hdr, struct net_device *orig_dev)
> > +{
> > + struct net_device *dev = rt->rt_vpls_dev;
> > + struct mpls_entry_decoded dec;
[...]
> > + dec = mpls_entry_decode(hdr);
>
> we can get dec.bos from mpls stack as a function param, no need to
> decode the mpls header again.
I didn't do that for 2 reasons:
- mpls_entry_decode is a very small inlined function
- for future OAM GAL label support, it will need to decode a 2nd label
> > +static const struct net_device_ops vpls_netdev_ops = {
> > + .ndo_init = vpls_dev_init,
> > + .ndo_open = vpls_open,
> > + .ndo_stop = vpls_close,
> > + .ndo_start_xmit = vpls_xmit,
> > + .ndo_change_mtu = vpls_change_mtu,
> > + .ndo_get_stats64 = ip_tunnel_get_stats64,
> > + .ndo_set_rx_mode = vpls_set_multicast_list,
> > + .ndo_set_mac_address = eth_mac_addr,
> > + .ndo_features_check = passthru_features_check,
> > +};
>
> can you add .ndo_get_stats64 function ?
It uses the same stats as ip tunnels:
+ .ndo_get_stats64 = ip_tunnel_get_stats64,
I don't think copypasting improves code quality here(???)
Cheers,
-David
