| Message ID | 1467827996-32547-4-git-send-email-simon.horman@netronome.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Jul 6, 2016 at 10:59 AM, Simon Horman <simon.horman@netronome.com> wrote: > * Set skb protocol based on contents of packet. I have observed this is > necessary to get actual protocol of a packet when it is injected into an > internal device e.g. by libnet in which case skb protocol will be set to > ETH_ALL. > I am not sure what do yo mean by ETH_ALL. I could not find it in the kernel. anyways, Can we fix libnet to set skb->protocol field correctly? The change is introducing overhead for every packet received on internal port. > * Set the mac_len which has been observed to not be set up correctly when > an ARP packet is generated and sent via an openvswitch bridge. > My test case is a scenario where there are two open vswtich bridges. > One outputs to a tunnel port which egresses on the other. > > The motivation for this is that support for outputting to layer 3 (non-tap) > GRE tunnels as implemented by a subsequent patch depends on protocol and > mac_len being set correctly on receive. > The commit msg and the change does not match anymore. > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > --- > v11 > * Do not set mac_len. > Instead of relying on mac_len follow-up patches now > use skb_unset_mac_header() > > v10 > * Set mac_len > > v9 > * New patch > --- > net/openvswitch/vport-internal_dev.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c > index 434e04c3a189..32d8e94d9bff 100644 > --- a/net/openvswitch/vport-internal_dev.c > +++ b/net/openvswitch/vport-internal_dev.c > @@ -48,6 +48,9 @@ static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev) > { > int len, err; > > + skb->protocol = eth_type_trans(skb, netdev); > + skb_push(skb, ETH_HLEN); > +
On Thu, Jul 07, 2016 at 01:52:25PM -0700, pravin shelar wrote: > On Wed, Jul 6, 2016 at 10:59 AM, Simon Horman > <simon.horman@netronome.com> wrote: > > * Set skb protocol based on contents of packet. I have observed this is > > necessary to get actual protocol of a packet when it is injected into an > > internal device e.g. by libnet in which case skb protocol will be set to > > ETH_ALL. > > > I am not sure what do yo mean by ETH_ALL. I could not find it in the kernel. > anyways, Can we fix libnet to set skb->protocol field correctly? The > change is introducing overhead for every packet received on internal > port. It was quite a while ago since I wrote this patch and I don't recall what I meant by ETH_ALL. I now plan to drop this patch.
diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c index 434e04c3a189..32d8e94d9bff 100644 --- a/net/openvswitch/vport-internal_dev.c +++ b/net/openvswitch/vport-internal_dev.c @@ -48,6 +48,9 @@ static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev) { int len, err; + skb->protocol = eth_type_trans(skb, netdev); + skb_push(skb, ETH_HLEN); + len = skb->len; rcu_read_lock(); err = ovs_vport_receive(internal_dev_priv(netdev)->vport, skb, NULL);
* Set skb protocol based on contents of packet. I have observed this is necessary to get actual protocol of a packet when it is injected into an internal device e.g. by libnet in which case skb protocol will be set to ETH_ALL. * Set the mac_len which has been observed to not be set up correctly when an ARP packet is generated and sent via an openvswitch bridge. My test case is a scenario where there are two open vswtich bridges. One outputs to a tunnel port which egresses on the other. The motivation for this is that support for outputting to layer 3 (non-tap) GRE tunnels as implemented by a subsequent patch depends on protocol and mac_len being set correctly on receive. Signed-off-by: Simon Horman <simon.horman@netronome.com> --- v11 * Do not set mac_len. Instead of relying on mac_len follow-up patches now use skb_unset_mac_header() v10 * Set mac_len v9 * New patch --- net/openvswitch/vport-internal_dev.c | 3 +++ 1 file changed, 3 insertions(+)