On Thu, Jan 11, 2018 at 07:23:38PM +0100, Jakub Sitnicki wrote:
> On Wed, 27 Dec 2017 15:35:23 +0100
> Jakub Sitnicki <[email protected]> wrote:
> 
> > On Tue, 26 Dec 2017 09:48:05 -0800
> > Ben Pfaff <[email protected]> wrote:
> > 
> > > On Thu, Dec 21, 2017 at 10:43:06AM +0100, Jakub Sitnicki wrote:  
> > > > If we cannot talk to ovs-vswitchd process for some reason, e.g. it has
> > > > terminated prematurely, we want to fail the test as soon as possible.
> > > > 
> > > > Otherwise the test will likely fail later on due to ARP tables not being
> > > > populated, which will make the troubleshooting the failure harder.
> > > > 
> > > > Signed-off-by: Jakub Sitnicki <[email protected]>
> > > > ---
> > > >  tests/ofproto-macros.at |  7 ++++---
> > > >  tests/ovn.at            | 46 
> > > > +++++++++++++++++++++++-----------------------
> > > >  2 files changed, 27 insertions(+), 26 deletions(-)
> > > > 
> > > > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> > > > index 55a4e77..1fe0223 100644
> > > > --- a/tests/ofproto-macros.at
> > > > +++ b/tests/ofproto-macros.at
> > > > @@ -271,23 +271,24 @@ ovn_attach() {
> > > >      start_daemon ovn-controller || return 1
> > > >  }
> > > >  
> > > > -# ovn_populate_arp
> > > > +# OVN_POPULATE_ARP
> > > >  #
> > > >  # This pre-populates the ARP tables of all of the OVN instances that 
> > > > have been
> > > >  # started with ovn_attach().  That means that packets sent from one 
> > > > hypervisor
> > > >  # to another never get dropped or delayed by ARP resolution, which 
> > > > makes
> > > >  # testing easier.
> > > > -ovn_populate_arp() {
> > > > +ovn_populate_arp__() {
> > > >      for e1 in $arp_table; do
> > > >          set `echo $e1 | sed 's/,/ /g'`; sb1=$1 br1=$2 ip=$3 mac=$4
> > > >          for e2 in $arp_table; do
> > > >              set `echo $e2 | sed 's/,/ /g'`; sb2=$1 br2=$2
> > > >              if test $sb1,$br1 != $sb2,$br2; then
> > > > -                as $sb2 ovs-appctl tnl/neigh/set $br2 $ip $mac
> > > > +                as $sb2 ovs-appctl tnl/neigh/set $br2 $ip $mac || 
> > > > return 1
> > > >              fi
> > > >          done
> > > >      done
> > > >  }
> > > > +m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], 
> > > > [ignore])])    
> > > 
> > > Seems like a good idea.
> > > 
> > > However, why not just name the macro ovn_populate_arp, so that no
> > > changes are needed outside ofproto-macros.at?  
> > 
> > I tried to follow here what seems to be a convention in tests, that m4
> > macro names are all upper case.
> 
> Ben, so what would be your preference? Keeping the changes down to
> ofproto-macros.at or sticking with all-upper-case for m4 macros?
> 
> I'm happy to respin the patch as needed.

Sorry, this just fell off my radar.  I applied this to master.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to